TheRosettaFoundation / SOLAS-Match

Self-managed translation project interface
www.TheRosettaFoundation.org
GNU Lesser General Public License v3.0
12 stars 8 forks source link

Auto-follow new projects for listed admins #1254

Closed aquilax closed 7 years ago

aquilax commented 7 years ago

1245 Calls trackProject for user ids defined in conf.ini sectionsite.autofollow_admin_ids.

Note: the extra changes in conf.template.ini are removed trailing space.

RFR @alanbarrett

alanbarrett commented 7 years ago

@aquilax

Two things...

1) Could you set up a pull request against develop, this one is against master which I know you did not intend.

2) I have a worry about the behavior of the code in the following cased, could you tell me what the return value is from getAutoFollowAdminIds() in the following cases (it should be an empty array)...

a) no autofollow_admin_ids setting in the config file. b) autofollow_admin_ids= in the config file (i.e. no values as you have in the default config)

Explode generates an array of length one when given an empty string (which will be casted to 0)... I do not know if (a) or (b) give an empty string.

Alan.
aquilax commented 7 years ago

Hi @alanbarrett

  1. You are right for this one. Added a check for empty string.
  2. Will close this pull request and create new one targeting develop. Related to that, how often master is the target of a pull request? Since we mostly target develop, I'd suggest to make it the default branch if you agree.
alanbarrett commented 7 years ago

If we change to develop, will a git clone then pick the develop branch by default (I do not know if this is so), which is not what we want?

Alan.

On 21 November 2016 at 15:46, Evgeniy Vasilev notifications@github.com wrote:

Hi @alanbarrett https://github.com/alanbarrett

  1. You are right for this one. Added a check for empty string.
  2. Will close this pull request and create new one targeting develop. Related to that, how often master is the target of a pull request? Since we mostly target develop, I'd suggest to make it the default branch if you agree.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/TheRosettaFoundation/SOLAS-Match/pull/1254#issuecomment-261974902, or mute the thread https://github.com/notifications/unsubscribe-auth/AAlN_43QtNDyA9F-_VH3dFzt-QWmxH_aks5rAby8gaJpZM4K3kn9 .

aquilax commented 7 years ago

It shouldn't affect git. according to the documentation:

After the clone, a plain git fetch without arguments will update all the remote-tracking branches, and a git pull without arguments will in addition merge the remote master branch into the current master branch, if any (this is untrue when "--single-branch" is given; see below).

https://help.github.com/articles/setting-the-default-branch/:

The default branch is considered the base branch in your repository, against which all pull requests and code commits are automatically made, unless you specify a different branch.

alanbarrett commented 7 years ago

But it also says before this... "Clones a repository into a newly created directory, ..., and creates and checks out an initial branch that is forked from the cloned repository’s currently active branch." ...this "currently active branch" might be "develop".

However go ahead with your plan to make "develop" the default, I will add a specific "git checkout master" to the instruction in the Wiki so there can be no problem.

Alan.

On 21 November 2016 at 15:58, Evgeniy Vasilev notifications@github.com wrote:

It shouldn't affect git. according to the documentation https://git-scm.com/docs/git-clone:

After the clone, a plain git fetch without arguments will update all the remote-tracking branches, and a git pull without arguments will in addition merge the remote master branch into the current master branch, if any (this is untrue when "--single-branch" is given; see below).

https://help.github.com/articles/setting-the-default-branch/:

The default branch is considered the base branch in your repository, against which all pull requests and code commits are automatically made, unless you specify a different branch.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/TheRosettaFoundation/SOLAS-Match/pull/1254#issuecomment-261979133, or mute the thread https://github.com/notifications/unsubscribe-auth/AAlN__lFBobzMWhwWhzGOkpMt6vOdhP4ks5rAb-3gaJpZM4K3kn9 .

aquilax commented 7 years ago

Decided check instead of guessing https://github.com/aquilax/test.git .Seems like github indeed changes the head and the default becomes develop . Still worth it IMO.

alanbarrett commented 7 years ago

Yes please change.

Alan.

On 22 November 2016 at 04:14, Evgeniy Vasilev notifications@github.com wrote:

Decided check instead of guessing https://github.com/aquilax/test.git .Seems like github indeed changes the head and the default becomes develop . Still worth it IMO.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/TheRosettaFoundation/SOLAS-Match/pull/1254#issuecomment-262143324, or mute the thread https://github.com/notifications/unsubscribe-auth/AAlN_2rQjKB9C-C4JdBBdoptayJRR3rUks5rAmw8gaJpZM4K3kn9 .

alanbarrett commented 7 years ago

@aquilax I have changed default to develop for SOLAS-Match, not sure if you have rights to do that.

Alan.
aquilax commented 7 years ago

:+1: