-
Notifications
You must be signed in to change notification settings - Fork 34
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
ci: add CHANGELOG check and specific targets #456
Conversation
Since we are working with CI: could we also address #450? |
Sure! |
.github/workflows/changelog.yml
Outdated
steps: | ||
- uses: dangoslen/changelog-enforcer@v3 | ||
with: | ||
skipLabels: pipelines,tests,documentation,no-changelog |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'd use the previously-existing 'no changelog'
label for this. We could also delete that one and keep no-changelog
, but I think it's a bit better if it's aligned with other Miden repos.
|
||
# --- Check --------------------------------------------------------------------------------------- | ||
|
||
check: ## Builds the CLI binary and client library in release mode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should add the phony targets for these
* chore: fix build and serialize procedures as bytes * chore: Point node to next * Fix node config file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Thank you! I did leave a few small comments inline.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
.PHONY: check | ||
check: ## Builds the CLI binary and client library in release mode | ||
cargo check --release --features $(FEATURES_CLI) | ||
|
||
.PHONY: check-wasm | ||
check-wasm: ## Builds the client library for wasm32 | ||
cargo check --target wasm32-unknown-unknown --features idxdb,web-tonic --no-default-features --package miden-client |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this has already been merged, but I would do a small follow-up PR to update the descriptions of these commands (otherwise, they are identical to the build
commands).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, my bad. I created this PR to fix this. Will create an issue to link it too. Sorry.
closes #452
TO DO:
Update: This PR will also address #450