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

ci: Refactor scripts #60

Merged
merged 32 commits into from
Feb 14, 2025
Merged

ci: Refactor scripts #60

merged 32 commits into from
Feb 14, 2025

Conversation

febo
Copy link
Collaborator

@febo febo commented Feb 6, 2025

Problem

CI currently has limited checks and the publish.yml workflow is limited to a couple of crates.

Solution

Refactor the CI. This involved using a new set of supporting scripts, which can also be used from the command-line:

  • audit: audit dependencies of the workspace
  • build: build a crate with build-sbf (useful to detect issues with code under the target = solana configuration
  • clippy: run clippy
  • doc: linting of doc sections
  • format: run cargo fmt
  • hack: linting of feature powerset
  • lint: execute clippy && doc && hack in sequence
  • semver: semantic versioning validation
  • test: run cargo test

CI make use of these scripts to run:

  1. audit
  2. For each crate of the repository, it executes:
    • format
    • clippy
    • doc
    • hack
    • build
    • test

The publish.yml workflow was updated to work with any crate of the reposiroty and incorporates semver-checks based on the version level. If the version level selected for publishing violates semantic versioning, the publishing fails. For example, if the code to be published under a patch version contains breaking changes, the semver-check will fail and the publish workflow will abort.

Note

The code in main is currently failing the doc linting - #64 fixes these issues.

@febo febo force-pushed the febo/refactor-ci branch from c2ad80e to 6be0456 Compare February 6, 2025 14:47
@febo febo force-pushed the febo/refactor-ci branch from 8bb36fb to 20e4cfb Compare February 6, 2025 16:17
@febo febo force-pushed the febo/refactor-ci branch from 20e4cfb to 4f7ad78 Compare February 6, 2025 18:07
@febo febo marked this pull request as ready for review February 7, 2025 23:03
@febo febo requested a review from joncinque February 7, 2025 23:06
@febo
Copy link
Collaborator Author

febo commented Feb 8, 2025

Just realised that the "filter" is not as precise as it should be. For example, when the main pinocchio crate changes, all crates that depend on it should also go through the CI checks – this is not happening with the current filter.

Update: The conditional filter was removed.

Copy link
Collaborator

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

Nothing show-stopping, mostly small questions -- looks great!

Cargo.toml Outdated
@@ -17,5 +17,14 @@ repository = "https://github.com/anza-xyz/pinocchio"

[workspace.dependencies]
five8_const = "0.1.3"
pinocchio = { path = "sdk/pinocchio", version = ">= 0.6, <= 0.7" }
pinocchio-pubkey = { path = "sdk/pubkey", version = "0.2.1" }
pinocchio = { path = "sdk/pinocchio", version = ">= 0.6" }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is the version specified like that?

Copy link
Collaborator Author

@febo febo Feb 11, 2025

Choose a reason for hiding this comment

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

This is probably me not using the workspace dependencies correctly. I was trying to avoid bumping dependencies on other packages unnecessarily. For example, if pinocchio-pubkey can work with multiple versions of pinocchio and pinocchio has been bumped to 0.7, I still want to specify that pinocchio-pubkey works with anything from >= 0.6.

But perhaps this is not the correct way to do that.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah I see, let's just keep them all at the same version for now, I think it'll make things simpler in the long run.

@febo febo requested a review from joncinque February 14, 2025 10:12
Copy link
Collaborator

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

A couple more comments, then this can go in!

Cargo.toml Outdated
@@ -17,5 +17,14 @@ repository = "https://github.com/anza-xyz/pinocchio"

[workspace.dependencies]
five8_const = "0.1.3"
pinocchio = { path = "sdk/pinocchio", version = ">= 0.6, <= 0.7" }
pinocchio-pubkey = { path = "sdk/pubkey", version = "0.2.1" }
pinocchio = { path = "sdk/pinocchio", version = ">= 0.6" }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah I see, let's just keep them all at the same version for now, I think it'll make things simpler in the long run.

Copy link
Collaborator

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

Looks great!

@febo febo merged commit f2a765b into main Feb 14, 2025
2 of 9 checks passed
@febo febo deleted the febo/refactor-ci branch February 14, 2025 13:41
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