-
Notifications
You must be signed in to change notification settings - Fork 0
RM-37342: Remove check on code outside modules #110
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
Conversation
brouwers-tiobe
left a comment
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.
The intention of this ticket is not fully clear to me. My interpretation would be: code outside of a module is OK, but having additional modules within a file is not. Meaning the rule should only trigger on module declarations, and nothing else.
But the current implementation also explicitly checks for code outside of modules, implying 'it is not OK to have code outside of modules if there is more than one module in the file'.
This seems incorrect to me, because in either case it is not applicable:
- If there is only one module, code outside it is allowed (as per this ticket)
- If there is more than one module, the problem is the additional modules. Once the additional modules are moved out of the file, the other code outside the module is not problematic (as in point 1)
Perhaps it would be best to verify the requirement here.
|
On hold while awaiting a reply on the requirements. |
|
Discussed the requirements. The 'code outside module' check can be removed as discussed in #110 (review). As such, it has been removed and the tests have been updated to reflect that. Decided to leave the test code as it is. |
|
Also adapted the pull request to reflect the new changes. |
brouwers-tiobe
left a comment
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.
LGTM!
Conform the description in the ticket itself (RM-37342):
Also added a few more examples of what is considered to be OK:
As it is easier to cover multiple cases in individual files, the tests have switched from a single file-and-expected construct to a directory containing multiple files.