-
-
Notifications
You must be signed in to change notification settings - Fork 489
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
[9.0][MIG] website_sale_suggest_create_account: Upgrade to v9 #125
[9.0][MIG] website_sale_suggest_create_account: Upgrade to v9 #125
Conversation
Same comments about the commit than in #124 |
"version": "9.0.1.0.0", | ||
"category": "Website", | ||
"website": "http://www.antiun.com", | ||
"author": "Antiun Ingeniería S.L., " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please switch this to the new company Tecnativa
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copyrights too, or just this author key?
I'll also try to keep this in mind and apply the change in other things going forward.
c1101ae
to
05937c4
Compare
@pedrobaeza - Updates made. Because I was rebasing and destroying our conversation anyways, I went ahead and just squashed the fix commit into my original. I would love to hear if you have a strategy for removing that merge commit, but I can't figure out a way to combine the disparate parents without it. Side note - now that I'm looking at Github, comments aren't being collapsed after being handled anymore. Double edged sword because our conversations aren't erroneously destroyed on rebasing, but now verifying what was done just became incredibly more difficult 😦 |
Yeah, the comments not dissapearing seems a bit odd. Maybe they will dissapear when approving changes... Please remove also the merge commit and you can also squash all the Yajo's commits in one. |
@pedrobaeza - How do I remove the merge commit? These were disparate parents because the module was not on 9.0. Is there some sort of wizardry that I don't know about here? It's part of the migration instructions as well, so we should update if you've got something fancy up your sleeve. Will do on the squashing. |
Other times that I made the method where the module is not in branch, I didn't have a merge commit, so I'm not sure why it appears. Anyway, please try the new experimental mode, that also works when the module doesn't exist, and thus we can see if we switch the instructions to this one. |
05937c4
to
535110b
Compare
@pedrobaeza - Done That new process was really difficult, there were conflicts almost every step of the way. I feel like I was doing something wrong, but I repeated it a few times. Can you point me to where this development is happening? I don't see an issue in maintainer-tools? |
It happened in the mailing list (some prefer this channel because is more "general audience"): https://odoo-community.org/groups/contributors-15/contributors-44716?mode=thread&date_begin=&date_end= |
4e38759
to
1836de3
Compare
Now a new button appears "Log in and checkout". Module website_sale_require_login depends on this one and removes the button for checkout without creating an account. [9cd7c45] Add module website_sale_suggest_create_account.
1836de3
to
8fd4ecc
Compare
* Migrate website_sale_suggest_create_account to v9 * Update author to Tecnativa * Update ReadMe issues text to new template
8fd4ecc
to
6ee25de
Compare
Thanks @pedrobaeza - I'll mess around locally a bit more then bring it to that thread. For the moment I reset then did some hilariously ghetto cherry-picks off of the backup from my initial branch which seem to have achieved the intended results. |
Merging this one to speed up PR treatment. |
This is the other half of #89 - website_sale_suggest_create_account, which I submitted out of order 😆