Skip to content

build: npm lint to properly find files on Win OS #488

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
Merged
kfcampbell merged 1 commit into from
Jan 3, 2023
Merged

build: npm lint to properly find files on Win OS #488

kfcampbell merged 1 commit into from
Jan 3, 2023

Conversation

@mikedidomizio
Copy link
Contributor

@mikedidomizio mikedidomizio commented Dec 19, 2022 •
edited
Loading

I was on my Windows 10 machine going down the rabbit hole of GH OpenAPI spec typedefs and realized that the lint (prettier) functionality doesn't work on Windows machines for this repository.

My first thought was that it was broken all around but on non-Windows machines I realized it was working!

I have not created an issue for this as I had already figured it out before getting to the point of PR creation. If you'd like me to create an issue and link it to this PR just let me know.

Behavior

Before the change?

Changing the prettier files list being wrapped with single quotes to escaped double quotes. Before it would not find any files, which afterwards I found seems to catch others as well.

$ npm run lint

> @octokit/[email protected] lint
> prettier --check '{src,test,scripts}/**/*.{js,ts,json}' README.md package.json !src/generated/* !scripts/update-endpoints/generated/*

Checking formatting...
[error] No files matching the pattern were found: "'{src,test,scripts}/**/*.{js,ts,json}'".
All matched files use Prettier code style!

After the change?

Now prettier should work in both Windows and Unix environments.

$ npm run lint

> @octokit/[email protected] lint
> prettier --check "{src,test,scripts}/**/*.{js,ts,json}" README.md package.json !src/generated/* !scripts/update-endpoints/generated/*

Checking formatting...
All matched files use Prettier code style!

Other information

OS - Windows 10
Node - v16.13.0

Additional info

Pull request checklist

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • Added the appropriate label for the given change

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

  • Yes (Please add the Type: Breaking change label)
  • No

If Yes, what's the impact:

  • N/A

Pull request type

Please add the corresponding label for change this PR introduces:

  • Bugfix: Type: Bug
  • Feature/model/API additions: Type: Feature
  • Updates to docs or samples: Type: Documentation
  • Dependencies/code cleanup: Type: Maintenance

@mikedidomizio
Copy link
Contributor Author

mikedidomizio commented Dec 22, 2022

As far as setting the Type label I don't think I can do that as a non-maintainer of the repo.

@nickfloyd nickfloyd added the Type: Feature New feature or request label Dec 30, 2022
@kfcampbell
Copy link
Contributor

kfcampbell commented Jan 3, 2023

Confirming this still works on Linux environments.

Current master branch results of linting:

 sh$ npm run lint

> @octokit/[email protected] lint
> prettier --check '{src,test,scripts}/**/*.{js,ts,json}' README.md package.json !src/generated/* !scripts/update-endpoints/generated/*

Checking formatting...
All matched files use Prettier code style!

Current feature branch results of linting:

 sh$ npm run lint

> @octokit/[email protected] lint
> prettier --check "{src,test,scripts}/**/*.{js,ts,json}" README.md package.json !src/generated/* !scripts/update-endpoints/generated/*

Checking formatting...
All matched files use Prettier code style!

The only difference appears to be the ' to " change, which is desired.

@kfcampbell kfcampbell merged commit 447e1e1 into octokit:master Jan 3, 2023
@mikedidomizio mikedidomizio deleted the fix-lint branch January 3, 2023 19:58
@octokitbot
Copy link
Contributor

octokitbot commented Jan 11, 2023

🎉 This PR is included in version 8.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Reviewers

1 more reviewer

@kfcampbell kfcampbell kfcampbell approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

released Type: Feature New feature or request

Projects

🧰 Octokit Active
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@mikedidomizio @kfcampbell @octokitbot @nickfloyd