Hi,

I see in the merge history of the repository that there is reference to the master branch of github.com:developmentseed/openlayers_plus.

My question is, did you know there is also a 'd7' branch in that repository?

I have some patches to make the code in the 'd7' branch of github.com:developmentseed/openlayers_plus work well with recent drupal 7 releases, but that code is a bit different from your module, before I re-adapt the patches I'd like to discuss with you if it's not better to base the module on that 'd7' branch from developmentseed.

It would be nice to know also something else about the project history, things like: were the other branches in the original repository merged in the current module at drupal.org? I can see there were a 'js' and a 'blocktoggle' branches as well.

Just FYI I might be interested in co-maintaining the openlayers_plus module on drupal.org, if you agree that is.

Thanks,
Antonio

Comments

batje’s picture

I noticed that devseed branch only after i had started working on this one. By the time i had found out, I had a working version of the things i needed, and some more.
I know some things could be done differently and maybe even better. But I think by now we are in a stage where that has to be done the drupal patches way.
If you want to help with that long-term, then you would be welcome

ao2’s picture

Thanks for the prompt reply batje.

The main difference I see is in the behaviors php code, d7 branch on developmentseed does stuff like:

 /**
   * Override of options_form().
   */
  function options_form($defaults = array()) {
    $form = parent::options_form($defaults);
    $form['positioned'] = array(
      '#type' => 'checkbox',
      '#title' => t('Position tooltip over feature'),
      '#default_value' => $this->options['positioned'],
    );
    return $form;
  }

while you are doing:

 /**
   * Override of options_form().
   */
  function options_form($defaults) {
    return array(
      'positioned' => array(
        '#type' => 'checkbox',
        '#title' => t('Position tooltip over feature'),
        '#default_value' => isset($defaults['positioned']) ? $defaults['positioned'] : array(),
      ),
    );
  }

The two things are almost the same, at first I was wondering if the $form = parent::options_form($defaults); was a more correct way of doing it, but then I saw that the OpenLayers module itself does just like you are doing it, so maybe that's the way to go.

Other differences involve indenting style in the javascript files, not a big deal, I'd just say: let's be consistent with the OpenLayers module on those too.

davidserene’s picture

hi, we use two spaces for indentation throughout the module code and also use $defaults = array() in the options_form function for all the behaviors to fix a Strict warning error that manifests like:

Strict warning: Declaration of openlayers_plus_behavior_blockswitcher::options_form() should be compatible with openlayers_behavior::options_form($defaults = Array) in _registry_check_code() (line 3080 of /var/www/openlayers722/includes/bootstrap.inc).

davidserene’s picture

Status: Active » Needs review
davidserene’s picture

Status: Needs review » Closed (fixed)

We are currently doing a review of all the Openlayers_plus code using pareview