Welcome, Guest
Username: Password: Remember me

TOPIC: Procedure for Pull Requests in Github

Procedure for Pull Requests in Github 2 years 1 month ago #74869

  • tacman1123
  • tacman1123's Avatar
  • OFFLINE
  • LimeSurvey Team
  • Posts: 125
  • Karma: 1
Limesurvey is on github -- YEA!!! This is great news.

What's the "proper" procedure for submitting pull requests? In particular, should non-team developers create their own branch for each independent idea they have, or their own branch for their organization, or ...?

Is there a place to discuss how to implement ideas that would be logical to be part of the core?

For example, we're about to add our implementation of restricting the language dropdown to our subset of languages

We also have a patch that allows domain-specific config.php files, which would allow individual clients to have their own limesurvey site that all shares the same underlying code. It's only slightly related to the first patch, in that some installations will have different language lists.

So I have those two changes to make in my forked copy of Limesurvey. Before I do, I'd like to discuss the ideas with the LS team. My implementation gets the language list as a comma-delimited set of languages, and is defined in the config.php file -- but what should that global variable be called, to be consistent with other LS nomenclature? Should the "proper" way be to include the list in the database, like "default site language"? I'm sure doing it with some pre-approval of the LS developers gives it a better chance of being pulled into the core.

Now, those are simple changes that will probably have reasonable support for inclusion into the core. But how about bigger discussion items, like dropping the random survey ID and instead using sequential ids? Including 0 in the survey id (maybe starting with a non-zero to keep id's a consistent length).

Or even bigger, like dropping the whole SGQA field names and replacing them the field names derived from the question and answer codes? Or expanding the answer codes to more than 5 characters? Or using Twig or Smarty as a rendering engine? These are big items, all of which I would advocate for, but unlikely to be embraced by the whole team. Is there a place to have a discussion about these kind of things?

Fortunately, with git one can create a new branch easily, implement their own idea, then submit it back, but if it's rejected, they can continue using their own branch. A client requested a permissions system that would restrict users to a subset of answers, or even a subset of groups within a survey. (Specifically, they wanted regional managers to only have access to surveys in their region, and they wanted subject experts to only have access to questions related to their field, but not the entire 130-question survey). I doubt many people would support that level of permissions, especially with the current security implementation of groups (where a group is really just a pre-defined set of permissions, but if group permissions change, the older individual users in that group don't reflect the new permissions).

Oh, and I'd LOVE to add a field name prefix to group, so that instead of having child1_age, child1_birthplace, etc. for group one and child2_age, child2_birthplace for group two, I could have group prefixes of child1, child2 and then question codes within the group would be age, birthplace, and would be expanded out in the questions table accordingly, to be unique. Etc. I've been waiting until LimeSurvey was in github before even considering making these changes, but now that it is I'd love to be involved in development and getting some of them into the core.

Version 2 brings a whole new set of discussions, but this post is already too long. Could you post some guidance on the procedure for implementing these kind of changes and the procedure for getting them pulled?

Thanks (and really looking forward to contributing!)

Tac
Last Edit: 2 years 1 month ago by tacman1123. Reason: unfinished message, accidentally submitted when trying to insert a link.
The administrator has disabled public write access.

Re: Procedure for Pull Requests in Github 2 years 1 month ago #74921

  • c_schmitz
  • c_schmitz's Avatar
  • OFFLINE
  • LimeSurvey Team
  • Posts: 719
  • Thank you received: 91
  • Karma: 83
Hello,

yes the according procedure for non-team members would be to create your own branch, work with that and at some time in the future ask foryour changes to be merged with a pull request.

Note that the feature "Limit the language options dropdown on the list surveys page" is already implemented in Version 2.0 (branch Yii). After 1.92 there will be no further 1.9x versions (except for Plus releases)

For bigger discussions just create an according topic for each issue here in the 'Future features' forum. Also submit all your ideas to the idea tracker at ideas.limesurvey.org , which is another way people can comment on it or provide alternative suggestions for a proper solution.

The likeliness of any new features to be accepted into the core rises alot if you communicate alot and follow our advice :-)
Support us, too. Donate to the LimeSurvey project and help keep us going!
The administrator has disabled public write access.

Re: Procedure for Pull Requests in Github 2 years 1 month ago #74931

  • tacman1123
  • tacman1123's Avatar
  • OFFLINE
  • LimeSurvey Team
  • Posts: 125
  • Karma: 1
Thanks. Should eat minor (but complete) change be its own branch? I'm still new to github, so want to learn the right way for doing this.

I noticed in 2.0 that on my netbook I have to scroll during the installation, as key buttons are hidden. Should I make a separate branch of my fork called installation, make the fixes there, and submit it to you, then go back to my other branch which might be something like 'importer', which will allow the user to import a lss file via a url rather than just from the file system. Those are completely separate -- should they be separate branches, or should they be all in a yii_tac branch?

Yes, I want to do everything I can to get my suggestions into the core. When they can't fit into the core, I have some ideas about a "wrapper" to Limesurvey that may also be of interest to other who have customizations that are too client-specific to belong in the core, but I'll introduce that in another thread. I'm sure I'll be using the API a lot as soon as its ready.

Tac
The administrator has disabled public write access.

Re: Procedure for Pull Requests in Github 2 years 1 month ago #74932

  • c_schmitz
  • c_schmitz's Avatar
  • OFFLINE
  • LimeSurvey Team
  • Posts: 719
  • Thank you received: 91
  • Karma: 83
Depends, if the changes are completely independent (so do not affect the same source files) you should be able to do it in the same branch and create pull requests for specific revisions.

With lots of changes for a feature (affecting lots of files) it might be better in general to create separate branches.
Support us, too. Donate to the LimeSurvey project and help keep us going!
The administrator has disabled public write access.

Re: Procedure for Pull Requests in Github 2 years 1 month ago #74989

  • TMSWhite
  • TMSWhite's Avatar
  • OFFLINE
  • LimeSurvey Team
  • Posts: 759
  • Thank you received: 81
  • Karma: 36
tacman1123 wrote:
Or even bigger, like dropping the whole SGQA field names and replacing them the field names derived from the question and answer codes?

Expression Manager already does this. You can still use SGQA, but the preferred naming system is based on the question and answer code. Here is the syntax.
tacman1123 wrote:
Or expanding the answer codes to more than 5 characters?

That is an easy database update.
tacman1123 wrote:
Oh, and I'd LOVE to add a field name prefix to group, so that instead of having child1_age, child1_birthplace, etc. for group one and child2_age, child2_birthplace for group two, I could have group prefixes of child1, child2 and then question codes within the group would be age, birthplace, and would be expanded out in the questions table accordingly, to be unique.

This is a little tricker. Would this prefixing apply to all groups? If so, then either (a) all variables would need to use the full, group-prefixed name, (b) you'd need a group-level setting to specify whether to add the group-prefix, or (c) you'd need two aliases for the variables - try to find a group-prefixed value first; and if not, look for a non-group-previxed name. This last option might sound appealing, but it risks undetected name collisions.

It is also tricky for data analysis and export, since you want to ensure that your SPSS/R/SAS variable names are unique.

I proposed an alternate strategy here, which would let you create a variable name prefix for your repeating groups, and have the prefix be properly converted when you re-import the group. Although not stated there, if this option were used, you could create the group once, then when you import it, have a field letting you create 20 copies of the group (for example) in a single click. Personally, that would be a lot easier to implement, and I think it should solve your problem.

/Tom
The administrator has disabled public write access.

Re: Procedure for Pull Requests in Github 2 years 1 month ago #75003

  • c_schmitz
  • c_schmitz's Avatar
  • OFFLINE
  • LimeSurvey Team
  • Posts: 719
  • Thank you received: 91
  • Karma: 83
TMSWhite wrote:
tacman1123 wrote:
Or expanding the answer codes to more than 5 characters?

That is an easy database update.

No, it is not. It has many implications, from extending the length for each field in each response table, up to reduced number of columns (and so fewer columns in general and so only a reduced number of possible (sub-)questions) for MySQL databases. You really do NOT want to change that unless you want to change the whole way how responses are stored, which in return affect everything else response-related, like statistics, response browsing, conditions engine, quotas etc...
Support us, too. Donate to the LimeSurvey project and help keep us going!
The administrator has disabled public write access.

Re: Procedure for Pull Requests in Github 2 years 1 month ago #76271

  • petersf
  • petersf's Avatar
  • OFFLINE
  • Fresh Lemon
  • Posts: 1
  • Karma: 0
Thanks for the information.
The administrator has disabled public write access.

Re: Procedure for Pull Requests in Github 2 years 1 month ago #76540

  • jun9
  • jun9's Avatar
  • OFFLINE
  • Fresh Lemon
  • Posts: 15
  • Thank you received: 1
  • Karma: 0
Great! Just forked limesurvey on Github.
@tacman1123, I'd love to see that patch that allows domain-specific config.php files, which would allow individual clients to have their own limesurvey site that all shares the same underlying code.
The administrator has disabled public write access.

Re: Procedure for Pull Requests in Github 2 years 1 month ago #76551

  • DenisChenu
  • DenisChenu's Avatar
  • OFFLINE
  • Moderator Lime
  • Posts: 5850
  • Thank you received: 716
  • Karma: 222
jun9 wrote:
@tacman1123, I'd love to see that patch that allows domain-specific config.php files, which would allow individual clients to have their own limesurvey site that all shares the same underlying code.
You can have something like that:
$databasename       =   'limesurvey-default';
if ($_SERVER['HTTP_HOST']=="one.example.org"){
$databasename       =   'limesurvey-one';
}
if ($_SERVER['HTTP_HOST']=="two.example.org"){
$databasename       =   'limesurvey-two';
}
Here it's just for database name, but you can separate $imageurl, $uploadurl ... but in config-default.php.

Denis
Last Edit: 2 years 1 month ago by DenisChenu. Reason: add the )
The administrator has disabled public write access.
Moderators: ITEd
Time to create page: 0.191 seconds
Donation Image