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 rustfmt to CI workflow #66

Open
str4d opened this issue Jan 8, 2022 · 3 comments
Open

Add rustfmt to CI workflow #66

str4d opened this issue Jan 8, 2022 · 3 comments
Labels
A-CI Area: Continuous Integration

Comments

@str4d
Copy link
Contributor

str4d commented Jan 8, 2022

#63 adds a workflow to check that the test vectors are up-to-date, and also hard-codes the test vectors. Once #65 is implemented, we'll want the hard-coded Rust test vectors to be nicely formatted, so we should add rustfmt to the CI workflow to ensure this.

@str4d
Copy link
Contributor Author

str4d commented Jan 8, 2022

Planned workflow change:

diff --git a/.github/workflows/test_vectors.yml b/.github/workflows/test_vectors.yml
index e2184b9..d53adf5 100644
--- a/.github/workflows/test_vectors.yml
+++ b/.github/workflows/test_vectors.yml
@@ -30,8 +30,20 @@ jobs:
       - name: Install dependencies
         run: poetry install --no-root
 
+      - uses: actions-rs/toolchain@v1
+        with:
+          toolchain: stable
+          components: rustfmt
+        if: matrix.kind == 'rust'
+
       - name: Regenerate test vectors
         run: ./regenerate.sh ${{ matrix.kind }} ${{ matrix.extension }}
 
+      - name: Check formatting
+        uses: actions-rs/cargo@v1
+        with:
+          command: fmt
+        if: matrix.kind == 'rust'
+
       - name: Verify there are no changes
         run: test -z "$(git status --porcelain)"

@daira
Copy link
Contributor

daira commented Feb 8, 2022

Shouldn't regenerate.sh run rustfmt (when generating Rust)? Then the existing flow would just do the right thing.

@str4d
Copy link
Contributor Author

str4d commented Feb 9, 2022

That might end up being how we do this (though the above workflow change would still be necessary in order for CI to have rustfmt even be available). But that still won't work for the existing workflow, only after #65. The current Rust output is un-formattable, because it expects to be copy-pasted into a function (as I was doing with the first Rust test vectors we wrote for Sapling).

@daira daira added the A-CI Area: Continuous Integration label Apr 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-CI Area: Continuous Integration
Projects
None yet
Development

No branches or pull requests

2 participants