-
Notifications
You must be signed in to change notification settings - Fork 0
custom rule to check component usage correctly #1
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
base: master
Are you sure you want to change the base?
custom rule to check component usage correctly #1
Conversation
…sions are avoided
cosminj
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.
It is quite a large PR, and I've only gone through the first file.
| //work around to find web root | ||
| // initialize known sub folders of web root | ||
| //without this, to find the web root , we have to search for server/web.xml coldfusion configuration file | ||
| String[] knownRootSubDirs = {"api", "voucherengine", "components", "avisfuelcard", "commandchain", "ioc", "parrot", "paycard", "santam", "voucherpos", "access", "customTags", "common"}; |
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.
These should be configurable in an external given file or environment variable.
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.
👍 made a change where we can pass these values using properties . We have a default set of values now and can override the default set using java -Dknownsubdirs=api,voucherengine,components ...
|
Let's hold on these changes. Since cflint story is back to SRE backlog, I would like to revisit the story and see what makes sense most sense to do from the SRE team's perspective. |
jbutkus
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.
@tonym128 shall this be handed over to your team, as part of the wider effort?
|
In my direct conversation with Rohith, we agreed he was going to merge this and provide me a new jar file with these changes included. My impression was that there was not much work left on this. @rohith could you confirm? |
yes @modernuniverse . i have pushed an update for the review comments and we should be good to merge after this. |
implemented a custom rule to check if a component used is correctly named and it exists in the path.
The rule will apply if any of following component usage is found
The rule checks two things based on the configuration (.cflintrc)
It can also throw error if component does exist but the name used is not pascalcase.
we can avoid the invalid component usage by removing INVALID_COMPONENT_USAGE from cflintrc.
This PR contains some kind of integration tests and unit tests for possible combinations of component usage and all the tests