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

feat: Include version number in all --json based outputs #17780

Closed
wants to merge 4 commits into from
Closed

feat: Include version number in all --json based outputs #17780

wants to merge 4 commits into from

Conversation

kamilogorek
Copy link
Contributor

@kamilogorek kamilogorek commented Feb 14, 2023

Affected commands:

  • deno doc --json
  • deno info --json
  • deno lint --json
  • deno lint --json --rules
  • deno bench --json

Things to note:

  • [WILDCARD] could be replaced with [WILDCARD].[WILDCARD].[WILDCARD] for more accurate assertion
  • breaking: doc --json returns the output under nodes now
  • breaking: lint --json returns the output under rules now
  • missing bench --json test, as I couldn't get it running 🤔

Resolves #17774

cli/tools/doc.rs Outdated Show resolved Hide resolved
@kamilogorek
Copy link
Contributor Author

kamilogorek commented Feb 15, 2023

Ah, gotcha. Thought that maybe detecting deno version that generated the output is sufficient. Updated.

We could also move all magic numbers to const JSON_SCHEMA_VERSION: u8 = 1;

Actually, lets do this, so it's easier to modify in the future.

@bartlomieju bartlomieju added this to the 1.31 milestone Feb 15, 2023
@aapoalas
Copy link
Collaborator

Breaking changes in the --json commands are, well, breaking. Neither lint --json nor doc --json are marked as unstable anymore so at least formally this should be a 2.0 thing.

@dsherret dsherret modified the milestones: 1.31, 2.0 Feb 16, 2023
@dsherret
Copy link
Member

Yes, the original issue was for 2.0.

@aapoalas
Copy link
Collaborator

Needs rebase, and then of course have to wait for a major version change :)

@kamilogorek
Copy link
Contributor Author

Ping me once it's ready for 2.0, so I don't have to rebase multiple times :)

@kamilogorek kamilogorek closed this by deleting the head repository Apr 12, 2024
@bartlomieju
Copy link
Member

@kamilogorek is there a particular reason you deleted the fork repo? We're on the verge of Deno 2 and wanted to merge this PR. Should we open a new one then?

@kamilogorek
Copy link
Contributor Author

I might've gone too far pruning stale PRs on my GitHub account 😅 resubmitted as #25335

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.

Add version numbers to all --json output
4 participants