Move some user code to concerns #7860
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jamesspeake
approved these changes
May 13, 2024
There was a problem hiding this comment.
Nice. This makes perfect sense and isolates logic really clearly. Notice you've moved the patch from the smart group engine. Fully agree with this for making the code more understandable. Think we should bring this up at the backend code discussion - we've talked about engines before, but I think we need to make sure we're all in agreement when it comes to trying to bring these patches into the core codebase.
|
Oh, but you do have a linting issue |
|
Base automatically changed from
revert-7494-revert-7354-TAN-1106-update-email-vienna
to
master
May 14, 2024 15:48
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Concerns are not perfect. They do not really isolate the code but still separate it into different files, which makes it hard to connect the dots sometimes.
But in this case, these two pieces of logic (groups and roles) are not connected to the rest of the
Usercode. And more importantly, the length ofUserwent out of control. It was hard to grasp what it does in general, hard to navigate through its methods, and even hard to breathe just looking at it.