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.

Sign up for GitHub

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

Jump to bottom

CI: Skip install_solidus check on non-current version #5477

Merged
merged 1 commit into from
Nov 2, 2023
Merged

CI: Skip install_solidus check on non-current version #5477

merged 1 commit into from
Nov 2, 2023

Conversation

tvdeyen
Copy link
Member

@tvdeyen tvdeyen commented Nov 1, 2023

Summary

The installer always uses the current released minor version (4.2 time writing this), but the check runs on all pipeline runs, wasting CI time and precious resources. Also it fails anyway, because of conflicting dependencies.

So, we only run the check on main and latest minor now.

Checklist

Check out our PR guidelines for more details.

The following are mandatory for all PRs:

The following are not always needed:

  • 📖 I have updated the README to account for my changes.
  • 📑 I have documented new code with YARD.
  • 🛣️ I have opened a PR to update the guides.
  • ✅ I have added automated tests to cover my changes.
  • 📸 I have attached screenshots to demo visual changes.

@tvdeyen tvdeyen requested a review from a team as a code owner November 1, 2023 08:40
@tvdeyen tvdeyen requested review from elia and removed request for a team November 1, 2023 08:40
@github-actions github-actions bot added the changelog:repository Changes to the repository not within any gem label Nov 1, 2023
@tvdeyen tvdeyen force-pushed the skip-install-check branch from 7780358 to d6d9f89 Compare November 1, 2023 08:45
@tvdeyen
Copy link
Member Author

tvdeyen commented Nov 1, 2023 •
edited
Loading

Oh boy. Circle CI syntax is the most confusing ever. Maybe someone with a deeper knowledge can fix this? /c @elia

@tvdeyen tvdeyen force-pushed the skip-install-check branch from d6d9f89 to 48ec0b9 Compare November 1, 2023 19:56
@codecov Codecov
Copy link

codecov bot commented Nov 1, 2023 •
edited
Loading

Codecov Report

Merging #5477 (8d653a5) into main (6e981d3) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #5477   +/-   ##
=======================================
  Coverage   88.93%   88.93%           
=======================================
  Files         622      622           
  Lines       14909    14909           
=======================================
  Hits        13259    13259           
  Misses       1650     1650

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@tvdeyen tvdeyen force-pushed the skip-install-check branch from 48ec0b9 to 2c0da77 Compare November 1, 2023 20:11
@tvdeyen
Copy link
Member Author

tvdeyen commented Nov 1, 2023

Ok, I figured it out, but we need to enable triggers in order to determine if the event send from github is a pull request event so we can trigger the solidus installer build for pull request to the main or v4.2 branches.

@tvdeyen
CI: Skip install_solidus check on non-current version
8d653a5 
The installer always uses the current released minor version
(4.2 time writing this), but the check runs on all pipeline
runs, wasting CI time and precious resources. Also it fails
anyway, because of conflicting dependencies.

So, we only run the check on main and latest minor now.
@tvdeyen tvdeyen force-pushed the skip-install-check branch from 2c0da77 to 8d653a5 Compare November 2, 2023 07:40
@tvdeyen
Copy link
Member Author

tvdeyen commented Nov 2, 2023 •
edited
Loading

I now configured it such that it only runs on main and v4.2 branches, but this does not include pull requests to these branches. Boy, I would like us to use github actions instead of circle ci...

kennyadsl
kennyadsl approved these changes Nov 2, 2023
View reviewed changes
Copy link
Member

@kennyadsl kennyadsl left a comment

Choose a reason for hiding this comment

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

It makes sense, this assumes that no one will install Solidus starting from older versions, and I think it's a safe assumptions.

@tvdeyen tvdeyen merged commit 00031a2 into solidusio:main Nov 2, 2023
4 checks passed
@tvdeyen tvdeyen deleted the skip-install-check branch November 2, 2023 09:42
@tvdeyen tvdeyen mentioned this pull request Nov 2, 2023
Merged
@tvdeyen tvdeyen added backport-v3.4 Backport this pull-request to v3.4 backport-v4.0 Backport this pull-request to v4.0 backport-v4.1 Backport this pull-request to v4.1 backport-v4.2 Backport this pull-request to v4.2 labels Nov 2, 2023
This was referenced Nov 2, 2023
Merged
Closed
Merged
Merged
@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 2, 2023

💔 Some backports could not be created

Status Branch Result
❌ v3.4 Backport failed because of merge conflicts

You might need to backport the following PRs to v3.4:
- Merge pull request #5459 from tvdeyen/revert-5367
✅ v4.0
✅ v4.1
✅ v4.2

Manual backport

To create the backport manually run:

backport --pr 5477

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 2, 2023

💔 Some backports could not be created

Status Branch Result
❌ v3.4 Backport failed because of merge conflicts

You might need to backport the following PRs to v3.4:
- Merge pull request #5459 from tvdeyen/revert-5367
✅ v4.0
✅ v4.1
✅ v4.2

Manual backport

To create the backport manually run:

backport --pr 5477

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 2, 2023

💔 Some backports could not be created

Status Branch Result
❌ v3.4 Backport failed because of merge conflicts

You might need to backport the following PRs to v3.4:
- Merge pull request #5459 from tvdeyen/revert-5367
✅ v4.0
✅ v4.1
❌ v4.2 An unhandled error occurred. Please see the logs for details

Manual backport

To create the backport manually run:

backport --pr 5477

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 2, 2023

💔 Some backports could not be created

Status Branch Result
❌ v3.4 Backport failed because of merge conflicts

You might need to backport the following PRs to v3.4:
- Merge pull request #5459 from tvdeyen/revert-5367
✅ v4.0
✅ v4.1
✅ v4.2

Manual backport

To create the backport manually run:

backport --pr 5477

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

@tvdeyen tvdeyen mentioned this pull request Nov 2, 2023
Merged
@tvdeyen
Copy link
Member Author

tvdeyen commented Nov 2, 2023

💚 All backports created successfully

Status Branch Result
✅ v3.4

Questions ?

Please refer to the Backport tool documentation

This was referenced Dec 15, 2023
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kennyadsl kennyadsl kennyadsl approved these changes

@elia elia Awaiting requested review from elia

Assignees
No one assigned
Labels
backport-v3.4 Backport this pull-request to v3.4 backport-v4.0 Backport this pull-request to v4.0 backport-v4.1 Backport this pull-request to v4.1 backport-v4.2 Backport this pull-request to v4.2 changelog:repository Changes to the repository not within any gem
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@tvdeyen @kennyadsl