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.

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 Github Action Super Linter #7637

Merged
merged 5 commits into from
Oct 15, 2020
Merged

Add Github Action Super Linter #7637

merged 5 commits into from
Oct 15, 2020

Conversation

WSLUser
Copy link
Contributor

@WSLUser WSLUser commented Sep 15, 2020

Summary of the Pull Request

This uses the templates from https://github.com/github/super-linter/tree/master/TEMPLATES currently. A future PR can add the necessary templates to the Windows Terminal repository and update the source of Templates following the README. Additionally we can add flags to explicitly choose the linters applicable to this code base but is not necessary.

References

#7513

PR Checklist

Detailed Description of the Pull Request / Additional comments

Per the README, this does not enforce any linting rules but rather outputs the suggestions in the build step, which are to be read by the PR submitter and Windows Terminal team to determine if they want to use the linting rule. C++ is currently not supported (Powershell, Json, Yaml, and Markdown will be the only things the linter checks for currently) but we could add our own custom support if desired in separate PR.

Validation Steps Performed

It successfully runs. Currently only shows the yaml file itself being linted in this PR as a test case. It will apply to new PRs once this is merged. We can lint existing code base but would require a separate PR and examining the code output (also requires updating the yaml file temporarily).

This uses the templates from https://github.com/github/super-linter/tree/master/TEMPLATES currently. A future PR can add the necessary templates to the Windows Terminal repository and update the source of Templates following the README. Additionally we can add flags to explicitly choose the linters applicable to this code base but is not necessary.
`2020-09-15 15:20:32 [WARN  ]   Report output folder (/github/workspace/super-linter.report) does NOT exist.`
@ghost ghost added Area-Build Issues pertaining to the build system, CI, infrastructure, meta Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. Issue-Task It's a feature request, but it doesn't really need a major design. Product-Meta The product is the management of the products. labels Sep 15, 2020
@WSLUser WSLUser mentioned this pull request Sep 15, 2020
6 tasks
@DHowett
Copy link
Member

DHowett commented Sep 15, 2020

This is cool! Looking at the log in the workflow, it seems to only have picked up the yml file. Is it supposed to detect files that are new in this PR, or will it lint everything?

@WSLUser
Copy link
Contributor Author

WSLUser commented Sep 15, 2020

Just new PRs for now. It does have some customization options we can use. This PR focuses on getting the basic support in. It only shows the yml file in this particular PR because this is the first PR with it being used in this repo.

@WSLUser
Copy link
Contributor Author

WSLUser commented Sep 23, 2020

Do we plan adding this in? Seems like there's plenty of doc PRs going around that could trivially be checked with this.

@miniksa
Copy link
Member

miniksa commented Sep 23, 2020

I can't find an example of what this does. Dustin apparently saw something, but I rooted around in the logs and don't see it reporting any particular issues. Can you screencap it?

On principle, I'm okay with this because I like automatic linting conceptually.

@WSLUser
Copy link
Contributor Author

WSLUser commented Sep 23, 2020

Show all checks and the very first one is the new linting build step called Lint Code Base. Their docs say use this name for automated sync to their repo

@DHowett
Copy link
Member

DHowett commented Oct 15, 2020

Thanks for the contribution!

@DHowett DHowett merged commit afcc930 into microsoft:master Oct 15, 2020
ghost pushed a commit that referenced this pull request Oct 15, 2020
For whatever reason, the super linter seems to think that any file it doesn't recognize is an EDITORCONFIG file. That means all our `cpp`, `hpp`, `h`, `resw`, `xaml`, etc files are going to get linted with different rules than the clang-format ones we already use. 

This PR disables the EDITORCONFIG linter, and has a minimal change to a cpp file to ensure that it's no longer linted by the action.

See also: 
* #7637 added this
* #7799 is blocked by this
* #7924 is blocked by this
DHowett pushed a commit that referenced this pull request Oct 15, 2020
Terminal ships with this dependency embedded, and it is not required that you install it separately. Since the link is broken, let's just remove it entirely.

* [x] fixes #7889 
* [x] related to #7917 (comment)
* [x] I work here
* [x] is a docs update

Additionally, update the markdown linter rules in the wake of #7637, because apparently that was never actually applied to any files, so now the onus is on the first person to touch any of our markdown files.
@DHowett
Copy link
Member

DHowett commented Oct 19, 2020

Since this one was filed on Sept. 15, I don't think it's eligible for Hacktoberfest.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Build Issues pertaining to the build system, CI, infrastructure, meta Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. Issue-Task It's a feature request, but it doesn't really need a major design. Product-Meta The product is the management of the products.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants