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

chore: expand Prettier glob to all files #701

Merged
merged 8 commits into from
Oct 12, 2023
Merged

Conversation

nschonni
Copy link
Contributor

The single quote glob expansion doesn't work on Windows. Instead of changing to the \" escapted double quote, I just used the dot expansion to run on everything and made the Husky hook match

Copy link
Member

@broofa broofa left a comment

Choose a reason for hiding this comment

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

@nschonni Thanks for the contribution!

Pulled. npm test and npm run lint both pass for me. CI checks are passing.

I don't have strong opinions about YAML formatting, but I know indent levels in YAML are importantish. The changes here appear to be benign, and I'm a fan of enforcing consistent formatting so this seems like a win.

@ctavan If this looks good to you, go ahead and merge.

@nschonni
Copy link
Contributor Author

I could override the Prettier config to use double quotes for Yaml files to reduce the diff, if that's preffered

@broofa
Copy link
Member

broofa commented Mar 22, 2023

I could override the Prettier config to use double quotes for Yaml files to reduce the diff, if that's preffered

Not necessary (at least, not on my account). I'm partial to single-quotes.

@ctavan
Copy link
Member

ctavan commented Mar 23, 2023

This looks good!

I've merged the PR that adds the ability to run bundlewatch and browser tests on branches, would you mind rebasing?

@nschonni
Copy link
Contributor Author

Rebased, but you'll need to add a safe to test label to the PR to trigger the browser tests

@ctavan ctavan added the safe to test PR deemed safe for testing using Github secrets. label Mar 23, 2023
@ctavan
Copy link
Member

ctavan commented Mar 23, 2023

Thanks a lot!

Bundlewatch returns an error, we haven't reconfigured it for the main branch (we renamed from master some time ago).

I may find time to fix this later today, but if you have a minute, you could try configuring the ci option in https://github.com/uuidjs/uuid/blob/main/bundlewatch.config.json in accordance with https://bundlewatch.io/#/reference/configuration?id=ci

@nschonni
Copy link
Contributor Author

@broofa I think you might need to remove and re-add the label to see the bundlewatch job triggered

@broofa broofa added safe to test PR deemed safe for testing using Github secrets. and removed safe to test PR deemed safe for testing using Github secrets. labels Mar 23, 2023
@ctavan ctavan added safe to test PR deemed safe for testing using Github secrets. and removed safe to test PR deemed safe for testing using Github secrets. labels Mar 23, 2023
@nschonni
Copy link
Contributor Author

Looks like the build worked, but there is still a dangling one for bundlewatch API check

@ctavan
Copy link
Member

ctavan commented Mar 23, 2023

Hmm, bundlewatch did run correctly now (https://github.com/uuidjs/uuid/actions/runs/4504866899/jobs/7929883055?pr=701) but still doesn't report status…

@nschonni
Copy link
Contributor Author

Might be this bundlewatch/bundlewatch#220 (comment)

@ctavan
Copy link
Member

ctavan commented Mar 23, 2023

Might be this bundlewatch/bundlewatch#220 (comment)

Wanna give it a try?

@broofa broofa added safe to test PR deemed safe for testing using Github secrets. and removed safe to test PR deemed safe for testing using Github secrets. labels Oct 11, 2023
@broofa broofa merged commit bc46e19 into uuidjs:main Oct 12, 2023
6 checks passed
@nschonni nschonni deleted the prettier-glob branch October 13, 2023 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
safe to test PR deemed safe for testing using Github secrets.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants