-
Notifications
You must be signed in to change notification settings - Fork 53
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
More code cleanup #200
More code cleanup #200
Conversation
This is dead code because toConvert is a List<String>, so it cannot contain an object of type Element.
Looks like this change is exposing a pre-existing permissions issue in CasC tests:
This was always an issue but was being silently ignored due to the use of the legacy file API. I'll temporarily add some |
And, of course, the error went away on a subsequent re-run. I have now removed the debugging instrumentation I added and am trying another test run. It is frustrating that these CasC tests are so flaky. I would suggest that someone with write access to this repository consider migrating to Azure Container Instances, which offer a more deterministic build environment. |
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.
Looks great, thanks+
A series of code cleanup changes for this repository. Most of these changes fix IDE warnings from IntelliJ. I also reformatted the Javadocs and fixed several typos.
I kept each commit separate. Each commit message should be self-explanatory, but if anything is unclear I am happy to provide further explanation. Also, feel free to revert any specific cleanup commits that are unwanted.