Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add summary comment to some rules with short description #1087

Merged
merged 4 commits into from
Oct 29, 2018
Merged

Add summary comment to some rules with short description #1087

merged 4 commits into from
Oct 29, 2018

Conversation

pedrocodacy
Copy link
Contributor

@pedrocodacy pedrocodacy commented Oct 23, 2018 •
edited by bergmeister
Loading

Fixes #1083

Added some summary comments in /Rules/*.cs files. This will allow Codacy to get proper pattern definitions for this tool's integration

@msftclas
Copy link

msftclas commented Oct 23, 2018 •
edited
Loading

CLA assistant check
All CLA requirements met.

@pedrocodacy pedrocodacy changed the title Add summary comment to some rules with short description #1083 Add summary comment to some rules with short description Oct 23, 2018
bergmeister
bergmeister approved these changes Oct 23, 2018
View reviewed changes
Copy link
Collaborator

@bergmeister bergmeister left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I will leave this PR open for a few days so that others can take a look as well and will merge then

@bergmeister bergmeister requested a review from rjmholt October 24, 2018 18:20
rjmholt
rjmholt approved these changes Oct 29, 2018
View reviewed changes
Copy link
Contributor

@rjmholt rjmholt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good generally.

The rule names (in the comment before the colon) are probably redundant if the class name already encodes them -- better to just describe what the rule does.

@bergmeister
Copy link
Collaborator

bergmeister commented Oct 29, 2018 •
edited
Loading

Yes, I agree that the rule name in the comment is a bit superfluous and could even be replaced by an actual reference in xml comment syntax but this would be a much bigger change. This PR makes comments at least consistent, I will therefore merge this PR to resolve the issue of the PR creator. We could take further improvements afterwards if desired.

@bergmeister bergmeister merged commit 9acf387 into PowerShell:development Oct 29, 2018
@rjmholt
Copy link
Contributor

rjmholt commented Oct 29, 2018

@bergmeister Didn't realise that was the existing convention :) Agreed on all fronts

bergmeister pushed a commit to bergmeister/PSScriptAnalyzer that referenced this pull request Mar 22, 2019
@pedrocodacy @bergmeister
Add summary comment to some rules with short description (PowerShell#…
aeb4fe2 
…1087)

* Add summary comment to some rules with short description

* correct typo

* add period

* add period
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bergmeister bergmeister bergmeister approved these changes

@rjmholt rjmholt rjmholt approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@pedrocodacy @msftclas @bergmeister @rjmholt