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

[4.2] Clean up relationship directory - move partials to inc subdirectory & rename relationship_select and repeatable_relation #4062

Merged
merged 5 commits into from
Jan 12, 2022

Conversation

tabacitu
Copy link
Member

@tabacitu tabacitu commented Jan 7, 2022

WHY

BEFORE - What was wrong? What was happening before this PR?

Inside the relationship directory we had a bunch of stuff all in one place. Both subfields and partials.

AFTER - What is happening after this PR?

The partials are all moved to a relationship.inc subdirectory, so all that's left in relationship is the subfields.

HOW

How did you achieve that, in technical terms?

  • Moved files to inc and added inc where they are referenced.
  • Put the code inside field_attributes into fetch_or_create since it was only used there.
  • Renamed relationship_select to select
  • Renamed repeatable_relation to entries

Is it a breaking change or non-breaking change?

Semi-breaking. Because one of the changes is inside the InlineCreateOperation. Though I doubt anybody has ever overriden that method.

How can we test the before & after?

No need to test, it was a few simple find&replace commands. But if you want, the Monsters CRUD (Create/Update) should work the same after this PR. No functionality was added or changed, just the directory structure changed from this:

Screenshot 2022-01-07 at 12 13 28

To this:

Screenshot 2022-01-07 at 12 13 52

@tabacitu tabacitu changed the title [4.2] Move relationship partials to inc subdirectory [4.2] Move relationship partials to inc subdirectory & rename relationship_select and repeatable_relation Jan 7, 2022
@tabacitu tabacitu changed the title [4.2] Move relationship partials to inc subdirectory & rename relationship_select and repeatable_relation [4.2] Clean up relationship directory - move partials to inc subdirectory & rename relationship_select and repeatable_relation Jan 7, 2022
@tabacitu tabacitu changed the title [4.2] Clean up relationship directory - move partials to inc subdirectory & rename relationship_select and repeatable_relation [4.2] Clean up relationship directory - move partials to inc subdirectory & rename relationship_select and repeatable_relation Jan 7, 2022
@tabacitu tabacitu requested a review from pxpm January 7, 2022 10:11
@tabacitu
Copy link
Member Author

tabacitu commented Jan 7, 2022

@pxpm I want to merge this TODAY, after our meeting. No point in postponing this, the more we do that the more chances of breaking changes when we do. In any case, I think git is smart enough to know these are the same files, just renamed - it seems so from the github diff. So I don't think they'll pose any problems.

But I'm going to wait until we talk today, to be sure you've submitted all changes to these files already.

@tabacitu
Copy link
Member Author

FYI - Github couldn't do the merge automatically, but my local Git could - I got absolutely no merge conflicts. So maybe we shouldn't be so scared of these file-renaming changes in the future 😉

@tabacitu tabacitu merged commit cebd2e2 into 4.2 Jan 12, 2022
@tabacitu tabacitu deleted the move-relationship-partials-to-inc-subdirectory branch January 12, 2022 10:18
@tabacitu tabacitu mentioned this pull request Jan 12, 2022
@tabacitu tabacitu mentioned this pull request Feb 4, 2022
Merged
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.

2 participants