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

build.sbt cleanup #2276

Merged
merged 3 commits into from
Jun 10, 2024
Merged

build.sbt cleanup #2276

merged 3 commits into from
Jun 10, 2024

Conversation

guymers
Copy link
Contributor

@guymers guymers commented Jun 9, 2024

Remove explicit ZIO test framework as it is built into sbt.

Remove snapshot resolvers to avoid depending on snapshots.

Remove scala-xml v1 workaround as it is no longer required.

Remove explicit ZIO test framework as it is built into sbt.

Remove snapshot resolvers to avoid depending on snapshots.

Remove scala-xml v1 workaround as it is no longer required.
@ghostdogpr
Copy link
Owner

I am not sure what happened but both your PRs seem to not have triggered the CI 🤔

@ghostdogpr ghostdogpr closed this Jun 10, 2024
@ghostdogpr ghostdogpr reopened this Jun 10, 2024
@kyri-petrou
Copy link
Collaborator

kyri-petrou commented Jun 10, 2024

I'm a bit conflicted about removing the snapshot repository from built.sbt. On one hand I agree with the reasoning, but on the other hand I often test zio / zio-query / zio-http snapshot versions via the caliban repo. Is there any way we can disable it only for CI when merging in main? Maybe via an envvar?

@ghostdogpr
Copy link
Owner

ghostdogpr commented Jun 10, 2024

I'm a bit conflicted about removing the snapshot repository from built.sbt. On one hand I agree with the reasoning, but on the other hand I often test zio / zio-query / zio-http snapshot versions via the caliban repo. Is there any way we can disable it only for CI when merging in main? Maybe via an envvar?

How about using Global Settings?

@guymers
Copy link
Contributor Author

guymers commented Jun 10, 2024

I am not sure what happened but both your PRs seem to not have triggered the CI 🤔

Yeah dunno, pushed again and now it seems to be running

@kyri-petrou
Copy link
Collaborator

kyri-petrou commented Jun 10, 2024

How about using Global Settings?

It's a nice workaround but in some cases it's useful to be able to use it for draft PRs as well (e.g., #2251). I'm not against removing it though, but if we can have a fairly decent way to keep it for dev/PRs and disable it for the main branch that'd be ideal AFAICT

@ghostdogpr
Copy link
Owner

How about using Global Settings?

It's a nice workaround but in some cases it's useful to be able to use it for draft PRs as well (e.g., #2251). I'm not against removing it though, but if we can have a fairly decent way to keep it for dev/PRs and disable it for the main branch that'd be ideal AFAICT

How about leaving it commented out then?

@kyri-petrou
Copy link
Collaborator

How about leaving it commented out then?

Yeah I think that should be fine 👍

@ghostdogpr ghostdogpr merged commit 09e00b2 into ghostdogpr:series/2.x Jun 10, 2024
10 checks passed
@guymers guymers deleted the build-cleanup branch June 10, 2024 07:17
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.

4 participants