-
-
Notifications
You must be signed in to change notification settings - Fork 83
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
Add Gitlab / Code Climate Output Format Support #210
Merged
TomasVotruba
merged 11 commits into
easy-coding-standard:main
from
Kenneth-Sills:kesills-gitlab-formatter
May 28, 2024
Merged
Add Gitlab / Code Climate Output Format Support #210
TomasVotruba
merged 11 commits into
easy-coding-standard:main
from
Kenneth-Sills:kesills-gitlab-formatter
May 28, 2024
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
…properties Whether or not a formatter supports progress bars is now encapsulated in the formatter class itself, as opposed to being scattered in a couple places and relying on maintainers remembering. Both this progress bar support and the formatter names are also implemented as static methods now, whose inclusion can be promised by their shared interface. The console output retains its `::NAME` constant because it needs to be used in the default parameters of a configuration constructor (being a default setting). Finally, there remains one hard-coded reference to the `json` output format, but only over in the `ListCheckersCommand`. I've left this alone since this is not the same type of formatter. Don't want to step on any toes. :)
I, personally, didn't realize this tool supported output formatting at first, and the existing issues seemed to have been closed without mentioning the addition of them. I'd just like to make it more obvious for new users!
My initial line of thinking about setting to 'blocker' was flawed. Not everyone uses this tooling the same way and they may want it to show up in the MR widget/diffs without _actually_ blocking. Since ECS focuses on code style standards instead of correctness issues, this seems an appropriate level.
This just makes testing easier, as seen in the existing test classes.
This is by no means comprehensive, but it at least confirms it respects configurations as expected, formatting is right for trivial cases, and that fingerprints are deterministic.
- Uncommenting an attribute I disabled for debugging. - Added an important clarifying comment.
…ormatter These should have been relative, but they were absolute. There's no correctness issue in the formatter itself, it just repeats the paths it's given. But test cases should generally be correct regardless.
public function getName(): string; | ||
public static function getName(): string; | ||
|
||
public static function hasSupportForProgressBars(): bool; |
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.
Pretty neat 👏
|
||
return [$simulatedErrors, $simulatedFixes]; | ||
} | ||
} |
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.
Well done 👏
Thank you for refactoring of the progress bar on formatter and feature with propper test 👏 This is joy to read, lets ship it |
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.
This adds the ability to generate Gitlab code quality reports (as well as the parent Code Climate format) for discovered ECS errors and unapplied fixes.
I've made sure to document this class extensively so both end users and future maintainers can understand any possible nuances, but please do let me know if it's too much for your tastes. The tests cover the major basis, though I wouldn't call this "fully covered".
While here, I also:
I did perform some testing and it seems to be working correctly from a small demo:
See Example Image
However, this did highlight the existence of a major Gitlab issue... That's not our problem, so if anyone opens a ticket about "Code Quality hasn't changed" relating to this feature, we can safely send them over there instead.