? Success

User tests: Successful: Unsuccessful:

avatar zero-24
zero-24
2 May 2015

This Backports the fix from the framework into the CMS:
joomla-framework/keychain@7e6488d

Thanks goes to @photodude here: #6672

avatar zero-24 zero-24 - open - 2 May 2015
avatar zero-24 zero-24 - change - 2 May 2015
Labels Added: ?
avatar zero-24 zero-24 - change - 2 May 2015
Labels Added: ?
avatar zero-24 zero-24 - change - 2 May 2015
Category External Library
avatar zero-24 zero-24 - change - 2 May 2015
Status New Pending
avatar photodude
photodude - comment - 2 May 2015

@zero-24 Thanks for the acknowledgment. Glad to see this fix going into place.

avatar Kubik-Rubik
Kubik-Rubik - comment - 9 May 2015

@zero-24 You PR can not be merged because it produces merge conflicts. Your staging is over 300 commits behind the current state! Please update your fork and the PR against the current state. Thank you!

avatar brianteeman
brianteeman - comment - 9 May 2015

@Kubik-Rubik you need to look at the method you are using to apply this. There is only one file changed in this PR and that file has not been changed for 4 months. The merge issue is due to the way YOU are doing it. There is nothing wrong with the PR or any need for it to be updated

avatar mbabker
mbabker - comment - 9 May 2015

Merge on review (and since it's the same fix applied in the Framework repo)

avatar mbabker mbabker - change - 9 May 2015
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2015-05-09 15:40:07
Closed_By mbabker
avatar mbabker mbabker - close - 9 May 2015
avatar mbabker mbabker - reference | fabc757 - 9 May 15
avatar mbabker mbabker - merge - 9 May 2015
avatar mbabker mbabker - close - 9 May 2015
avatar roland-d
roland-d - comment - 9 May 2015

@brianteeman I have tried to merge the same branch and I get the same errors. I am not doing this in any way different than the other PRs. Why should I? Perhaps I am doing it wrong as well and don't know how to do it. There are other ways to merge but we are trying to keep our log clean.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6883.

avatar brianteeman
brianteeman - comment - 9 May 2015

Well @mbabker managed to do it :)
On 9 May 2015 4:54 pm, "RolandD" notifications@github.com wrote:

@brianteeman https://github.com/brianteeman I have tried to merge the
same branch and I get the same errors. I am not doing this in any way
different than the other PRs. Why should I? Perhaps I am doing it wrong as
well and don't know how to do it. There are other ways to merge but we are

trying to keep our log clean.

This comment was created with the J!Tracker Application
https://github.com/joomla/jissues at issues.joomla.org/joomla-cms/6883
http://issues.joomla.org/tracker/joomla-cms/6883.


Reply to this email directly or view it on GitHub
#6883 (comment).

avatar zero-24 zero-24 - head_ref_deleted - 9 May 2015
avatar zero-24 zero-24 - head_ref_restored - 9 May 2015
avatar zero-24
zero-24 - comment - 9 May 2015

@roland-d @Kubik-Rubik hmm i guess the problem is that i have 2 accounts (zero-24 and zero24). One with push access and one without push access :smile:

This branche is 300+ commits behind staging
https://github.com/zero-24/joomla-cms/tree/patch-5

But this (is the branche of the patch)
https://github.com/zero24/joomla-cms/tree/patch-5

is ok. Sorry that this is confused.

avatar zero-24 zero-24 - change - 9 May 2015
Milestone Added:
avatar brianteeman
brianteeman - comment - 9 May 2015

That shouldnt make any difference. It is not reasonable to expect all pull
requests to always be up to date with current staging.
On 9 May 2015 18:56, "zero-24" notifications@github.com wrote:

@roland-d https://github.com/roland-d @Kubik-Rubik
https://github.com/Kubik-Rubik hmm i guess the problem is that i have 2
accounts (zero-24 and zero24). One with push access and one without push
access [image: :smile:]

This branche is 300+ commits behind staging
https://github.com/zero-24/joomla-cms/tree/patch-5

But this (is the branche of the patch)
https://github.com/zero24/joomla-cms/tree/patch-5

is ok. Sorry that this is confused.


Reply to this email directly or view it on GitHub
#6883 (comment).

avatar Kubik-Rubik
Kubik-Rubik - comment - 9 May 2015

@zero-24 Yes, this was the problem. We used the account that you use actively here... The "zero24" branch works without a problem.
@brianteeman Haha, yes, push the green button! :-D By the way, you can not know the way how we in the PLT do the commits properly (no, not the merge button at GitHub), so your comments are acceptable!

avatar mbabker
mbabker - comment - 9 May 2015

Just remember there are a few people not on PLT who may review/merge patches that should be kept in the loop on policy decisions...

avatar Kubik-Rubik
Kubik-Rubik - comment - 9 May 2015

@mbabker I'm talking about squashing all commits of the PR first in one on your local machine to avoid a mess in the history.

avatar mbabker
mbabker - comment - 9 May 2015

Not always necessary, especially when the PR is only a single commit (like this one), unless you're trying to avoid the "merged PR whatever" commits, then someone decided something and forgot to tell me.

avatar Kubik-Rubik
Kubik-Rubik - comment - 9 May 2015

@mbabker This is something we want to discuss at the PLT summit after JAB and establish a policy rule.

avatar zero-24 zero-24 - head_ref_deleted - 9 May 2015
avatar brianteeman
brianteeman - comment - 9 May 2015

As @mbabker said you need to make sure ALL people with commit rights know
On 9 May 2015 21:49, "Viktor Vogel" notifications@github.com wrote:

@mbabker https://github.com/mbabker This is something we want to
discuss at the PLT summit after JAB and establish a policy rule.


Reply to this email directly or view it on GitHub
#6883 (comment).

Add a Comment

Login with GitHub to post a comment