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.

Sign up for GitHub

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

Jump to bottom

N°7137 - DataSynchro: Remove "Organization" as default value for SynchroReplica->dest_class #551

Merged
merged 1 commit into from
Jan 11, 2024
Merged

N°7137 - DataSynchro: Remove "Organization" as default value for SynchroReplica->dest_class #551

merged 1 commit into from
Jan 11, 2024

Conversation

Hipska
Copy link
Contributor

@Hipska Hipska commented Sep 25, 2023 •
edited

@piRGoif piRGoif added the bug Something isn't working label Sep 25, 2023
@Hipska Hipska mentioned this pull request Sep 25, 2023
Draft
@piRGoif
Copy link
Contributor

piRGoif commented Sep 25, 2023

Hello,
The commit introducing the default value for SynchroReplica.dest_class is actually f3f2eb5, made back in 2011 (!!)
There are no explicit reason invoked in the commit message nor bug reference that could help :/

@Hipska
Copy link
Contributor Author

Hipska commented Sep 25, 2023

Hmm, right. But there are also a lot in between that don't have this as default value..

@piRGoif
Copy link
Contributor

piRGoif commented Sep 25, 2023

I don't see that. Which commit(s) are you refering to ?

@Hipska
Copy link
Contributor Author

Hipska commented Sep 25, 2023

So 2dfad12 was only 4 years ago and changed from not having a default value to having Organization. So the commits before that didn't have a default..

Nevermind, I just saw the commit before that did actually had this default as well, so you are right it is since f3f2eb5, updated the PR description.

@Molkobain
Copy link
Member

Accepted during technical, it makes no sense to have this value and null value is allowed anyway.

@Molkobain Molkobain self-assigned this Dec 5, 2023
@Molkobain Molkobain added this to the 2.7.11 milestone Jan 9, 2024
@Molkobain
Copy link
Member

Accepted during functional review. To be retrofitted in support/2.7.

@Molkobain
Copy link
Member

@Hipska can you modify the PR (and the branch if necessary) to target support/2.7?

@Hipska Hipska changed the base branch from develop to support/2.7 January 11, 2024 09:36
@Hipska
Copy link
Contributor Author

Hipska commented Jan 11, 2024

Done

@Molkobain
Copy link
Member

Molkobain commented Jan 11, 2024 •
edited

Thanks, I'll just wait for 2.7.10 to be released and I'll merge.

@Molkobain Molkobain changed the title fix(SynchroReplica): Remove default dest_class N°7137 - DataSynchro: Remove "Organization" as default value for SynchroReplica->dest_class Jan 11, 2024
@Molkobain Molkobain merged commit 7fffbb6 into Combodo:support/2.7 Jan 11, 2024
@Hipska Hipska deleted the fix/replica_default_class branch January 11, 2024 14:39
@Molkobain
Copy link
Member

Molkobain commented Jan 11, 2024 •
edited

Did you test it @Hipska ?
It didn't even pass the setup.

@Hipska
Copy link
Contributor Author

Hipska commented Jan 11, 2024 •
edited

Oh, it should probably be 'default_value' => '', then?

Don't know anymore why I removed that line.

@Molkobain
Copy link
Member

I know the fix, but I'm surprised you didn't test your branch.

@Hipska
Copy link
Contributor Author

Hipska commented Jan 11, 2024

No, I don't have any 2.7 systems running anymore..

@Molkobain
Copy link
Member

Fixed in 7a0a4e3

Molkobain pushed a commit that referenced this pull request Apr 4, 2024
@Hipska @Molkobain
N°7137 - DataSynchro: Remove "Organization" as default value for Sync…
21a9a94 
…hroReplica->dest_class (#551)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Molkobain Molkobain Molkobain approved these changes

Assignees

@Molkobain Molkobain

Labels
bug Something isn't working
Projects
None yet
Milestone
2.7.11
3 participants
@Hipska @piRGoif @Molkobain