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: add a comprehensive dfx json reference #2361

Merged

Conversation

sesi200
Copy link
Contributor

@sesi200 sesi200 commented Jul 20, 2022

Description

Adds dfx schema, a command that prints the current schema for dfx.json. Also added a workflow to keep a copy of the output up to date at docs/dfx-json-schema.json so that the docs can reference the current schema.

First part of SDK-287

How Has This Been Tested?

  • manually
  • workflow checks that docs/dfx-json-schema.json is up to date
  • e2e checks that dfx schema produces valid json

Checklist:

  • The title of this PR complies with Conventional Commits.
  • I have edited the CHANGELOG accordingly.
  • I have made corresponding changes to the documentation.

@sesi200 sesi200 requested a review from a team as a code owner July 20, 2022 09:29
@smallstepman
Copy link
Contributor

smallstepman commented Jul 20, 2022

I think building a separate github actions workflow that updates the JSON schema file will suffice, I'm not sure how having e2e test would then be useful. The workflow could run on each push for each PR, and do something along these lines:

if ! diff -q old.json new.json &>/dev/null; then
  git add new.json
  git commit -m "" 
  git push
fi

The workflow could be made :required so it is impossible to merge without completing it first.

@sesi200 sesi200 marked this pull request as draft July 20, 2022 15:35
@sesi200 sesi200 marked this pull request as ready for review July 21, 2022 14:32
sesi200 added 2 commits July 21, 2022 16:41
…site' of github.com:dfinity/sdk into SDK-287-add-a-comprehensive-dfx-json-reference-to-docs-site
@@ -0,0 +1,45 @@
name: Update Docs
on:
pull_request:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
pull_request:
pull_request

the : is unnecessary here

~/.cargo/git
target
key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }}
- name: Install Rust
Copy link
Contributor

Choose a reason for hiding this comment

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

more of a fyi, cause I'm not 100% certain here (help @adamspofford-dfinity), but I think this step is unnecessary since we have rust-toolchain.toml file - cargo should be able to use the correct version here

to verify, you might want to try rustc --version instead and see if its 1.60.0

@smallstepman
Copy link
Contributor

smallstepman commented Jul 21, 2022

ooof chicken and egg problem https://github.com/dfinity/sdk/runs/7451711903?check_suite_focus=true
you probably want to touch docs/dfx-json-schema.json and git add it.

nvm, i see its there

Co-authored-by: Marcin Nowak-Liebiediew <marcin.liebiediew@dfinity.org>
@mergify mergify bot merged commit fce523e into master Jul 21, 2022
@mergify mergify bot deleted the SDK-287-add-a-comprehensive-dfx-json-reference-to-docs-site branch July 21, 2022 17:56
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.

2 participants