Skip to content

Remove ci.sh script to run all from workflow #298

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

Merged
merged 1 commit into from
Jul 10, 2025

Conversation

hug-dev
Copy link
Member

@hug-dev hug-dev commented Jul 10, 2025

The ci.sh script was used in the main script, but only to execute cargo test. It was also used in the nightly workflow but it's not needed since the same steps are run in the merge workflow. Also prevents the nightly from failing.

@hug-dev hug-dev force-pushed the remove-ci-script branch from 6e931b9 to 93913aa Compare July 10, 2025 08:41
The ci.sh script was used in the main script, but only to execute cargo
test. It was also used in the nightly workflow but it's not needed since
the same steps are run in the merge workflow.

Signed-off-by: Hugues de Valon <[email protected]>
@hug-dev hug-dev force-pushed the remove-ci-script branch from 93913aa to 1d47788 Compare July 10, 2025 08:47
@hug-dev hug-dev marked this pull request as ready for review July 10, 2025 08:52
@hug-dev hug-dev requested review from Jakuje and wiktor-k July 10, 2025 08:52
@wiktor-k
Copy link
Collaborator

One thing that the ci.sh did that's currently not done is testing bindings generation (cargo build --features generate-bindings). Do you think it's worth to test that or should we skip that? The CI has additional benefit that due to matrix jobs we can check if generating bindings for a variety of targets work 🤔

@hug-dev
Copy link
Member Author

hug-dev commented Jul 10, 2025

True but I thought that the "Check for errors" workflow would check it with the --all-features flag in the following command?

cargo check --target ${{ matrix.target }} --all-features --workspace --all-targets

@wiktor-k
Copy link
Collaborator

Oh, right. So that should already be covered. Good point! 😅

Copy link
Collaborator

@Jakuje Jakuje left a comment

Choose a reason for hiding this comment

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

lgtm

@hug-dev hug-dev merged commit 868a6d8 into parallaxsecond:main Jul 10, 2025
43 checks passed
@hug-dev hug-dev deleted the remove-ci-script branch July 10, 2025 13:49
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.

3 participants