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

tidy: update dependencies for scala 3 migration #489

Merged
merged 4 commits into from
Jun 3, 2023
Merged

tidy: update dependencies for scala 3 migration #489

merged 4 commits into from
Jun 3, 2023

Conversation

i10416
Copy link
Contributor

@i10416 i10416 commented May 30, 2023 •
edited
Loading

part of #468
notable changes

  • replace type arguments with _ as compiler cannot check them
  • update scala parser combinators to 2.x(, which causes some weird error though...😰)
  • drop codacy sbt plugin as it is abandoned

i10416
i10416 commented May 30, 2023
View reviewed changes
modules/core/shared/src/main/scala/cron4s/parsing/parser.scala
(fieldWithAny(daysOfMonth) <~ blank) ~!
(field(months) <~ blank) ~!
fieldWithAny(daysOfWeek)
(field(seconds) <~! blank) ~!
Copy link
Contributor Author

@i10416 i10416 May 30, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Avoid back tracking

i10416
i10416 commented May 30, 2023
View reviewed changes
tests/shared/src/test/scala/cron4s/validation/EveryNodeValidatorSpec.scala
@@ -29,7 +29,7 @@ class EveryNodeValidatorSpec extends Cron4sPropSpec with ValidatorPropSpec {

private[this] def check[F <: CronField](implicit
unit: CronUnit[F],
enum: Enumerated[CronUnit[F]]
`enum`: Enumerated[CronUnit[F]]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

enum is keyword in Scala 3.

@i10416
Copy link
Contributor Author

i10416 commented May 30, 2023

Hello, could you take a look at this PR when you're free?

@alonsodomin

@i10416 i10416 force-pushed the tidy/update-dependencies-for-scala-3-migration branch from e8548f1 to 9699840 Compare May 30, 2023 17:01
@i10416 i10416 force-pushed the tidy/update-dependencies-for-scala-3-migration branch from 9699840 to 2899db5 Compare May 30, 2023 17:05
@alonsodomin
Copy link
Owner

Thanks a bunch for this, let's get it merged as soon as the CI checks pass

@alonsodomin
Copy link
Owner

@i10416 You need to run sbt githubWorkflowGenerate to generate a new Github Actions workflow file as the plugin updates have made the current one obsolete.

@i10416
Copy link
Contributor Author

i10416 commented Jun 3, 2023

@alonsodomin Sorry, I just updated workflow files.

@i10416 i10416 force-pushed the tidy/update-dependencies-for-scala-3-migration branch 2 times, most recently from 3ee9948 to 148abe0 Compare June 3, 2023 08:34
@i10416
fix: tweak build to pass ci build phase
a939063 
- use 8 as a version identifier  because version identifier must be 8 instead of 1.8
- use LTS java version because actions no longer support java 15
- need scoverage 2.x to support latest scala versions, which requires a
  hack to avoid dependency conflict of scala-xml
@i10416 i10416 force-pushed the tidy/update-dependencies-for-scala-3-migration branch from 148abe0 to a939063 Compare June 3, 2023 09:00
@i10416
Copy link
Contributor Author

i10416 commented Jun 3, 2023 •
edited
Loading

updates

  • use 8 as a version identifier because version identifier must be 8 instead of 1.8
  • use LTS java version because actions no longer support java 15
  • need scoverage 2.x to support latest scala versions, which requires a
    hack to avoid dependency conflict of scala-xml

confirmed build error resolved.
https://github.com/i10416/cron4s/actions/runs/5162846340/jobs/9300820781?pr=2

NOTE: The error seems to be reletad to #418

@i10416
Copy link
Contributor Author

i10416 commented Jun 3, 2023 •
edited
Loading

So sorry I forgot updating sbt-scoverage here.

https://github.com/i10416/cron4s/actions/runs/5163322053

@i10416 i10416 mentioned this pull request Jun 3, 2023
Closed
@alonsodomin
Copy link
Owner

thanks a lot for the effort, I know this was a bit of tedious one

@alonsodomin alonsodomin merged commit ca3f5c9 into alonsodomin:master Jun 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@i10416 @alonsodomin