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 support for JSON5 as a configuration file. #1118

Merged
merged 1 commit into from
Nov 7, 2024

Conversation

jwir3
Copy link
Contributor

@jwir3 jwir3 commented Nov 5, 2024

This adds support for JSON5 as a configuration file language, allowing for an extension to JSON that supports comments and other features.

Fixes CAP-2357.
Fixes #1114.

📦 Published PR as canary version: 11.18.0--canary.1118.11728711367.0

✨ Test out this PR locally via:

npm install [email protected]
# or 
yarn add [email protected]

This adds support for JSON5 as a configuration file language, allowing
for an extension to JSON that supports comments and other features.

Fixes CAP-2357.
Fixes #1114.
@jwir3 jwir3 requested a review from a team November 5, 2024 17:46
@jwir3 jwir3 added the patch Auto: Increment the patch version when merged label Nov 5, 2024
Copy link
Contributor

github-actions bot commented Nov 5, 2024

📦 Package Size: 5408 KB

@jwir3 jwir3 force-pushed the jwir3/cap-2357-allow-json5-in-cli branch from 66075c8 to c4086bf Compare November 5, 2024 17:49
Copy link

codacy-production bot commented Nov 5, 2024

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
Report missing for 89e6d251 100.00% (target: 80.00%)
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (89e6d25) Report Missing Report Missing Report Missing
Head commit (aa5e4fc) 7138 4861 68.10%

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#1118) 20 20 100.00%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

Codacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more

Footnotes

  1. Codacy didn't receive coverage data for the commit, or there was an error processing the received data. Check your integration for errors and validate that your coverage setup is correct.

Copy link
Contributor

@codykaup codykaup left a comment

Choose a reason for hiding this comment

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

Code makes sense to me. This feels more like a minor rather than a patch to me since it's not a bug. Also, don't forget to add release/skip-release!

package.json Outdated Show resolved Hide resolved
node-src/lib/getConfiguration.ts Outdated Show resolved Hide resolved
node-src/lib/getConfiguration.ts Outdated Show resolved Hide resolved
node-src/lib/getConfiguration.test.ts Outdated Show resolved Hide resolved
@jwir3
Copy link
Contributor Author

jwir3 commented Nov 6, 2024

Code makes sense to me. This feels more like a minor rather than a patch to me since it's not a bug. Also, don't forget to add release/skip-release!

Ok, I'll switch to minor. Just to be clear - we want to release this, right? There's no benefit to skip-release except in the case of things that are dev-only, like tests?

@jwir3 jwir3 added minor Auto: Increment the minor version when merged and removed patch Auto: Increment the patch version when merged labels Nov 6, 2024
@codykaup
Copy link
Contributor

codykaup commented Nov 6, 2024

Code makes sense to me. This feels more like a minor rather than a patch to me since it's not a bug. Also, don't forget to add release/skip-release!

Ok, I'll switch to minor. Just to be clear - we want to release this, right? There's no benefit to skip-release except in the case of things that are dev-only, like tests?

Yep, I would release this one. I only use skip-release if it's either internal or I'm stacking changes to release at once.

@jwir3 jwir3 force-pushed the jwir3/cap-2357-allow-json5-in-cli branch from 2ff92c0 to 36eeaab Compare November 6, 2024 20:57
@jwir3 jwir3 added the release Auto: Create a `latest` release when merged label Nov 6, 2024
@jwir3 jwir3 requested a review from codykaup November 6, 2024 21:03
Copy link
Contributor

@codykaup codykaup left a comment

Choose a reason for hiding this comment

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

Just some more things I noticed now that this supports .json5!

node-src/lib/getConfiguration.ts Outdated Show resolved Hide resolved
@jwir3 jwir3 force-pushed the jwir3/cap-2357-allow-json5-in-cli branch 2 times, most recently from cd6e838 to d36d95e Compare November 7, 2024 17:09
Copy link
Contributor

@codykaup codykaup left a comment

Choose a reason for hiding this comment

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

This looks great, nice work!

@jwir3 jwir3 force-pushed the jwir3/cap-2357-allow-json5-in-cli branch from d36d95e to aa5e4fc Compare November 7, 2024 17:47
@jwir3 jwir3 added this pull request to the merge queue Nov 7, 2024
Merged via the queue into main with commit 820acae Nov 7, 2024
19 checks passed
@jwir3 jwir3 deleted the jwir3/cap-2357-allow-json5-in-cli branch November 7, 2024 18:02
@chromatic-ci-bot
Copy link
Collaborator

🚀 PR was released in v11.17.0 🚀

@chromatic-ci-bot chromatic-ci-bot added the released Verdict: This issue/pull request has been released label Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor Auto: Increment the minor version when merged release Auto: Create a `latest` release when merged released Verdict: This issue/pull request has been released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow comments in chromatic.config.json
3 participants