Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert merge from master #10277

Closed
wants to merge 94 commits into from
Closed

Conversation

seamuslee001
Copy link
Contributor

No description provided.

adixon and others added 30 commits November 30, 2016 15:00
…g per payment processor for recurring contributions
The <value> tag only appeared once in all the schema defintion files thus
it seemed to me to be a typo of the (no longer used) <values> tag.

Also, I searched CRM/Core/CodeGen/Specification.php for ["']value["'] and
found no occurences which makes me think this tag is not parsed.
There were 72 occurences of <type>CheckBox</type>
and only 1 occurence of <type>Checkbox</type>

So it seemed like "Checkbox" was a typo. Thus I changed it
to "CheckBox"
There were 59 occurences of <type>TextArea</type>
and only 2 occurences of <type>TexArea</type>

So it seemed like "TexArea" was a typo. Thus I changed it
to "TextArea"
There were 5 occurences of <change>
and only 1 occurence of <modify>

So it seemed like <modify> was a typo. Thus I changed it
to <change>
…is called on activities.

I limited this to activities as there are only 2 separate values & I thought it would keep it readable.
… function to run the SQL piece. This means that sourceSQLFile no longer has an 'isQueryString' variable, which was used to pass an SQL query in the filename parameter.
(NFC) CRM-20308 Fix Test due to not truncating civicrm_email table
CRM-20459 replace last instances where CRM_Core_OptionGroup::getValue…
CRM-20459 replace all instances where CRM_Core_OptionGroup::getValue …
This will affect new installs only per discussion on the ticket. Motivation dependent, we could add a method to convert on existing installs.

Note some fields, like event_start_date might have a case for being in a non-timezone aware format
CRM-20459 replace all instances where CRM_Core_OptionGroup::getValue …
CRM-20435: Replace SQL-based activityContact creation with DAO-based approach.
…eSQLFile_rework

CRM-20428: refactored sourceSQLFile
CRM-9683 implement timezone support for CiviMail.
@seamuslee001
Copy link
Contributor Author

seamuslee001 commented Apr 28, 2017

@eileenmcnaughton i think you broke the RC with this c85b067 because it moved it to .20. I believe this correctly reverts that commit pining @davejenx as well

@seamuslee001
Copy link
Contributor Author

I think we should probably revert this as well 3c8e8aa

@eileenmcnaughton
Copy link
Contributor

ug did I - that seems like a very big commit!

@seamuslee001
Copy link
Contributor Author

yeh

@eileenmcnaughton
Copy link
Contributor

Yuck yuck yuck - expletitives! I did have this moment when something wierd happened but!!!

@seamuslee001
Copy link
Contributor Author

@eileenmcnaughton i have reverted the other merge from master just as a precautionary measure

@eileenmcnaughton
Copy link
Contributor

oh - so I merge the wrong way around???

@seamuslee001
Copy link
Contributor Author

@eileenmcnaughton i think so, I think you were hoping to merge 4.7.19 -> master not master -> 4.7.19

@eileenmcnaughton
Copy link
Contributor

This seems to be the PR - but is says into master #10266

@seamuslee001
Copy link
Contributor Author

I think you were trying to fix jenkins issues so this c85b067 and 3c8e8aa are the problem commits

@eileenmcnaughton
Copy link
Contributor

screenshot 2017-04-29 00 57 29

That's so confusing - I see 2 separate things on this screen

@seamuslee001
Copy link
Contributor Author

so the overall PR was merged into master but that commit was merging master branch into 4.7,19 and i think its only showing what elements of that commit make up part of the PR i think

@seamuslee001
Copy link
Contributor Author

Jenkins test this please

@eileenmcnaughton
Copy link
Contributor

OK - that seems like you are right to me - I think we should let @guanhuan @totten @colemanw know & merge this on pass

If we feel like normality is not returned we should consider skipping a release.

Offending PR link is #10266

@seamuslee001
Copy link
Contributor Author

Jenkins test this please

@totten
Copy link
Member

totten commented Apr 28, 2017

After we fix this, I expect folks will habitually resume the practice of forward-porting RC fixes to master -- but wouldn't that cause a sort of reverberation where the revert ultimately applies to master?

Feels like we need to either:

  1. Do a revert like Revert merge from master #10277; then all forward-porting via cherry-pick?
  2. Do a rewrite of the history (force-push) so that this never happened?

@totten
Copy link
Member

totten commented Apr 28, 2017

That's so confusing - I see 2 separate things on this screen
...
Offending PR link is #10266

I think it's confusing because #10266 didn't actually cause the problematic merge, but it's one of the few places where the problematic merge is visible. Pay close attention to the text of the merge message:

Theory:

  • Something like this was done via CLI: git checkout 4.7.19-rc ... git merge master ... git push origin 4.7.19-rc.
  • (Plausible? Well, some folks use a personal dev-branch with the same name as the canonical branch, and then it's easy to mix-up: git push myuser 4.7.19-rc and git push origin 4.7.19-rc.)
  • Later on, 4.7.19 rc #10266 aimed at forward-porting 4.7.19-rc changes to master. The errant merge commit appears in the history, but it's basically a nullop (because master already has master commits), so it hardly registers as a blip in the Github UI.

@eileenmcnaughton
Copy link
Contributor

I did have a moment when I was trying to merge something into 4.7.19 & suddenly it was already there - I checked my repos to see if I had pushed to 4.7.19rc but it's set up via https - which means entering a username & password

upstream https://github.com/civicrm/civicrm-core.git

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.