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

Fix CI #11

Merged
merged 2 commits into from
Sep 19, 2023
Merged

Fix CI #11

merged 2 commits into from
Sep 19, 2023

Conversation

WillB97
Copy link
Contributor

@WillB97 WillB97 commented Sep 17, 2023

Use repo version of sr-tools

@WillB97 WillB97 requested a review from a team September 17, 2023 18:55
@PeterJCLaw
Copy link
Member

PeterJCLaw commented Sep 17, 2023

I appreciate the desire here, however using the development version of the tools has a number of consequences -- most notably that still-in-development behaviours could end up committed to the inventory repo and thus foisted upon users who may not have (or even be able) to install that version of the tooling.

(I'm also slightly surprised that this works -- the reason that we've not published a new version of the tools is that their build stack is somewhat broken)

Copy link
Member

@RealOrangeOne RealOrangeOne left a comment

Choose a reason for hiding this comment

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

I'm afraid I agree with Peter here. Pinning like this is likely to cause us more headaches in future. Even if what's there now is stable enough for us to use, it might not always be true (although IMO main should still be reasonably stable, even if not "production-grade").

I see 3 possible ways forward:

  1. Fix sr.tools upstream, ship a release, which should then fix CI here (preferred).
  2. Pin to a specific commit. Sure, it's far from ideal, but at least this way we're at a stable-ish version which will more reliably work, even if we do this whilst going with Option 1.
  3. Just disable CI. Ignoring CI errors (and communicating that's what people should do) is objectively worse than having no CI.

Copy link
Member

@RealOrangeOne RealOrangeOne left a comment

Choose a reason for hiding this comment

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

I like this totally temporary solution.

@WillB97 WillB97 merged commit 444a1de into main Sep 19, 2023
1 check passed
@WillB97 WillB97 deleted the CI-fix branch September 19, 2023 19:07
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.

5 participants