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

Adds sbt-typelevel #4

Merged
merged 1 commit into from
May 3, 2022
Merged

Adds sbt-typelevel #4

merged 1 commit into from
May 3, 2022

Conversation

tlazaro
Copy link
Collaborator

@tlazaro tlazaro commented May 1, 2022

Added sbt-typelevel, mostly following the instructions on the site, trying to stick to the defaults and typelevel/paiges#449.

  • Set the license to Apache
  • JVM only for now
  • Scala 2.13 only but prepared the build to support multiple versions, just add to crossScalaVersions at the top
  • Adds NoPublishPlugin to all modules so there is no publishing yet
  • Adds doc project but kept empty

@tlazaro tlazaro mentioned this pull request May 2, 2022
@tlazaro
Copy link
Collaborator Author

tlazaro commented May 2, 2022

@johnynek if you could take a look and let me know what to do about these AWS failures. Maybe just ignoring these tests for now?

Also, should we be working on this branch or master?

@johnynek
Copy link
Owner

johnynek commented May 2, 2022

oh weird... somehow this branch got set as the default. We should change that.

So you can comment out the test, but the bigger question is why are we hitting any AWS initialization code? There is probably something impure going on that is breaking things that will bite us elsewhere.

It may be as easy as finding something and making the initialization lazy val but the "correct" approach would be to use IO better.

Lastly, cats-effect got much better in cats-effect 3 (fixing the need for Blocker to be passed around everywhere, but it isn't available for 2.11, so if we were to use this in a 2.11 repo, it is something to think of).

@johnynek
Copy link
Owner

johnynek commented May 2, 2022

but to be clear: just comment out anything you need to get this green, then we will deal with the better fix later.

@johnynek
Copy link
Owner

johnynek commented May 2, 2022

I've set the default branch to master, but let's keep this pr against this weird one, and then just force push the weird one to master.

@tlazaro
Copy link
Collaborator Author

tlazaro commented May 2, 2022

Perfect, will pick this up later today.

I would like this repo to work with 2.11 so we can use this at work without trouble but we would have to leave so much behind... What a pain :/

@tlazaro
Copy link
Collaborator Author

tlazaro commented May 2, 2022

Ah BTW, what license should be used? I set it to Apache to try it out.

@tlazaro
Copy link
Collaborator Author

tlazaro commented May 3, 2022

Basically in S3App we just need to make these lazy:

lazy val s3Client = s3.AmazonS3ClientBuilder.defaultClient()
lazy val awsIO = new AWSIO(s3Client)

@tlazaro
Copy link
Collaborator Author

tlazaro commented May 3, 2022

Not sure what Mergify is about, let me know what else needs to be done in this PR.

Copy link
Owner

@johnynek johnynek left a comment

Choose a reason for hiding this comment

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

looks good, but big diff...

@johnynek johnynek merged commit 744d4eb into oscar/frdcandle May 3, 2022
@johnynek
Copy link
Owner

johnynek commented May 3, 2022

mergify can automatically merge some changes (e.g. those from scala-steward that are green).

@tlazaro tlazaro deleted the tlazaro/sbt-typelevel branch September 10, 2022 18:09
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