Skip to content

Event addon migration - #70

Merged
paulius-sladkevicius merged 2 commits into
OCA:8.0from
paulius-sladkevicius:event_mig
Sep 17, 2014
Merged

Event addon migration#70
paulius-sladkevicius merged 2 commits into
OCA:8.0from
paulius-sladkevicius:event_mig

Conversation

@paulius-sladkevicius

Copy link
Copy Markdown

Related #29
Remake to point to 8.0 branch

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The custom is to rename the abandoned column name ('note' in this case) to its legacy name in the pre-script, then refer to this name in the post-script. While this is only necessary when the column type changes while keeping its name, I think it would be good to do this for any abandoned column that is touched by OpenUpgrade.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@StefanRijnhart I thought if fields 'oldname' func parameter was used we don't need to make it legacy by OpenUpgrade, so I don't see point to create additional legacy field when we don't touch e.g. 'note' field. Otherwise we should do that for all fields that are using 'oldname'.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If oldname is used, it is even worse because then you will not have the column anymore when you try to access it in this code....

@bwrsandman

Copy link
Copy Markdown

This PR will need to be rebased.

@StefanRijnhart

Copy link
Copy Markdown
Member

👍

@legalsylvain

Copy link
Copy Markdown
Contributor


Hi,
I don't use this module, so maybe my question is not relevant.
In 7.0: there is a main_speaker_id (res.partner) : 'DEL';
in 8.0: there is a organizer_id (res.partner) : 'NEW;
OK, a speaker is not an organizer but maybe it could be better to rename main_speaker_id to organizer_id, to avoid to loose data for the users that use that module, even if the semantic of the fields has changed.

What do you think ?

(Otherwise: LGTM)

@paulius-sladkevicius

Copy link
Copy Markdown
Author

@legalsylvain I think it will only confuse user, if somebody will need to keep main_speaker_id then better to inherit event.event and add such field.

@paulius-sladkevicius

Copy link
Copy Markdown
Author

Could somebody help to close this PR?
Sorry I can blame myself, because I'm not active now.

@legalsylvain

Copy link
Copy Markdown
Contributor

Ow. sorry @komsas , I forgot this review. Thanks for the explanation. and 👍

@bwrsandman

Copy link
Copy Markdown

Needs to be rebased again

@pedrobaeza

Copy link
Copy Markdown
Member

👍

@paulius-sladkevicius

Copy link
Copy Markdown
Author

@bwrsandman where I need to rebase it? Can you give more details?

@arthru

arthru commented Sep 17, 2014

Copy link
Copy Markdown

@komsas it seems that there are conflicts between your branch and the official one.

You simply need to git pull --rebase from the official branch and then git push (possibly with --force).

@paulius-sladkevicius

Copy link
Copy Markdown
Author

@arthru thanks, I didn't notice in first place that there are conflicts. Travis green text gave me green light ;-)

paulius-sladkevicius added a commit that referenced this pull request Sep 17, 2014
[ADD] Migration scrips for event addon
@paulius-sladkevicius
paulius-sladkevicius merged commit 407fcdb into OCA:8.0 Sep 17, 2014
@paulius-sladkevicius
paulius-sladkevicius deleted the event_mig branch September 17, 2014 16:17
GuillemCForgeFlow pushed a commit to ForgeFlow/OpenUpgrade that referenced this pull request Oct 10, 2024
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.

6 participants