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 Vale linter #2406

Merged
merged 13 commits into from
Apr 16, 2023
Merged

Add Vale linter #2406

merged 13 commits into from
Apr 16, 2023

Conversation

wesley-dean-flexion
Copy link
Contributor

Fixes #2401

Proposed Changes

  1. Add Vale prose checker to spell

Readiness Checklist

Author/Contributor

  • Add entry to the CHANGELOG listing the change and linking to the corresponding issue (if appropriate)
  • If documentation is needed for this change, has that been included in this pull request

^ none of this is done -- just testing to see what CI/CD does (at @nvuillam's request) ; will update before making PR ready to be merged

Reviewing Maintainer

  • Label as breaking if this is a large fundamental change
  • Label as either automation, bug, documentation, enhancement, infrastructure, or performance

@nvuillam
Copy link
Member

nvuillam commented Mar 1, 2023

@wesley-dean-flexion To solve the own MegaLinter issue, you can download the artifacts in the job page -> https://github.com/oxsecurity/megalinter/actions/runs/4308028827

Within it, you'll find an updated cspell.json that you can copy paste into your repo folder to overwrite the existing one in .github/linters :)
(check these are not real typos before committing ^^ )

@nvuillam
Copy link
Member

nvuillam commented Mar 1, 2023

And if you did not try yet, to see issues before the looooong CI job, you can test locally :) https://megalinter.io/v6/contributing/#execute-the-tests-locally-visual-studio-code

@wesley-dean-flexion
Copy link
Contributor Author

@nvuillam will do, but it may not be today

@nvuillam
Copy link
Member

nvuillam commented Mar 1, 2023

You need also to build to generate test class and update dockerfile, + to update the changelog ^^

https://megalinter.io/v6/contributing/#without-write-access

(except if @echoix 's magic command works and can do that from here ^^ )

@nvuillam
Copy link
Member

nvuillam commented Mar 1, 2023

@nvuillam will do, but it may not be today

That's the rule with open-source: no deadline, you do what you can when you can :)

@echoix
Copy link
Collaborator

echoix commented Mar 1, 2023

Let's try it, it would be a good debugging to see if it was only dependabot that didn't work

@echoix
Copy link
Collaborator

echoix commented Mar 1, 2023

/build

Command run output
Build command workflow started.
Installing dependencies
Running script ./build.sh
Build command workflow completed without updating files.

@wesley-dean-flexion
Copy link
Contributor Author

I merged the main branch from the upstream repository and attempted to run a build yesterday (15 March) and the build is still failing for me. I haven't walked through the command to see what's actually going on yet.

I'm attempting to track down what may be throwing an exit code of 2. I tested all of the curl / wget command locally and they work just fine (i.e., only 2xx and 3xx HTTP response codes) so I suspect that the errant program is something else.

Next, I'm looked at ${ML_THIRD_PARTY_DIR} and see that here, it has a value of /third-party/misspell . The command after the declaration was mkdir -p ${ML_THIRD_PARTY_DIR} (so, mkdir -p /third-party/misspell) while several commands later, there's sh .${ML_THIRD_PARTY_DIR} which would interpolate as sh ./third-party/misspell (which would be fine if the working directory was /). I don't know if that's an issue or not; however, I would recommend consistency with or without the . before the variable. I don't know the context well enough to know if one or the other (or both) are correct. (this is where I suspect someone to reply, "PRs are welcome!")

That's all I have for now. Apologies for taking so long to get back to this..

This is the error message I see:

buildx failed with: ERROR: failed to solve: process "/bin/sh -c dotnet tool install --global Microsoft.CST.DevSkim.CLI --version 0.7.104     && curl -sSfL https://raw.githubusercontent.com/anchore/syft/main/install.sh | sh -s -- -b /usr/local/bin     && wget --tries=5 -q -O - https://raw.githubusercontent.com/aquasecurity/trivy/main/contrib/install.sh | sh -s -- -b /usr/local/bin     && sfdx plugins:install @salesforce/sfdx-scanner     && npm cache clean --force || true     && rm -rf /root/.npm/_cacache     && ./coursier install scalafix --quiet --install-dir /usr/bin && rm -rf /root/.cache     && ML_THIRD_PARTY_DIR=\"/third-party/misspell\"     && mkdir -p ${ML_THIRD_PARTY_DIR}     && curl -L -o ${ML_THIRD_PARTY_DIR}/install-misspell.sh https://git.io/misspell     && sh .${ML_THIRD_PARTY_DIR}/install-misspell.sh     && find ${ML_THIRD_PARTY_DIR} -type f -not -name 'LICENSE*' -delete -o -type d -empty -delete     && find /tmp -path '/tmp/tmp.*' -type f -name 'misspell*' -delete -o -type d -empty -delete     && dotnet tool install --global TSQLLint     && mkdir -p /opt/kics/assets" did not complete successfully: exit code: 2

@Kurt-von-Laven
Copy link
Collaborator

This reminds me of #2378. I suspect this may be technical debt we inherited from Super-Linter that currently none of us knows about unfortunately.

@nvuillam
Copy link
Member

@wesley-dean-flexion FYI I added new commits on your branch to try to make the PR pass :)

@nvuillam nvuillam marked this pull request as ready for review April 15, 2023 23:18
@nvuillam nvuillam self-requested a review as a code owner April 15, 2023 23:18
@nvuillam
Copy link
Member

It works locally... I hope the CI jobs will work too, I'm very excited to have Vale in MegaLinter, even if it means that we'll have to rewrite half the documentation because of my french-globbish 🤣

Copy link
Member

@nvuillam nvuillam left a comment

Choose a reason for hiding this comment

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

I'll merge once the CI is ok, many thanks @wesley-dean-flexion :)

@nvuillam nvuillam enabled auto-merge (squash) April 15, 2023 23:21
@nvuillam nvuillam changed the title Initial commit of Vale linter Add Vale linter Apr 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Vale prose style checker
4 participants