Skip to content

Only run inliner in CI - #2889

Merged
mergify[bot] merged 1 commit into
slick:mainfrom
mdedetrich:only-run-inliner-in-ci
Feb 6, 2024
Merged

Only run inliner in CI#2889
mergify[bot] merged 1 commit into
slick:mainfrom
mdedetrich:only-run-inliner-in-ci

Conversation

@mdedetrich

@mdedetrich mdedetrich commented Feb 6, 2024

Copy link
Copy Markdown
Contributor

The Scala 2 inliner can cause issues when developing locally due to incremental compilation, i.e. from https://docs.scala-lang.org/overviews/compiler-options/optimizer.html#in-brief

Don’t enable the optimizer during development: it breaks incremental compilation, and it makes the compiler slower. Only enable it for testing, on CI, and to build releases.

This PR only enables the inliner in CI and also prints a log statement statement that its enabled to make it clear.

@mdedetrich

mdedetrich commented Feb 6, 2024

Copy link
Copy Markdown
Contributor Author

Working as expected, i.e. see https://github.com/slick/slick/actions/runs/7792797152/job/21251456956?pr=2889#step:7:30

@github-actions

github-actions Bot commented Feb 6, 2024

Copy link
Copy Markdown
Contributor

Incompatible changes

slick

3 changes since 3.5.0-pre.26.b6e6680c

Code changes

Incompatibility Symbol Problem
Backward slick.compat.collection.package MissingClassProblem

class slick.compat.collection.package does not have a correspondent in current version
Backward slick.compat.collection.package$ MissingClassProblem

object slick.compat.collection.package does not have a correspondent in current version
Backward slick.compat.collection.package$JavaConverters$ MissingClassProblem

object slick.compat.collection.package#JavaConverters does not have a correspondent in current version

@nafg

nafg commented Feb 6, 2024

Copy link
Copy Markdown
Member

Would prefer to keep one scala version pattern match but if you disagree it's ok

@mdedetrich

mdedetrich commented Feb 6, 2024

Copy link
Copy Markdown
Contributor Author

Would prefer to keep one scala version pattern match but if you disagree it's ok

Sure, I will also update the log statement

@mdedetrich
mdedetrich force-pushed the only-run-inliner-in-ci branch from 335f555 to 1a3d5c1 Compare February 6, 2024 00:47
@mdedetrich

mdedetrich commented Feb 6, 2024

Copy link
Copy Markdown
Contributor Author

PR updated with a pattern match, also updated log statement to mention which sbt module the inliner is being enabled for

@nafg

nafg commented Feb 6, 2024

Copy link
Copy Markdown
Member

It's still 2 pattern matches ;)

@mdedetrich

mdedetrich commented Feb 6, 2024

Copy link
Copy Markdown
Contributor Author

It's still 2 pattern matches ;)

Oh you mean you want to combine into a single match. Sure I can do that, have to reverse the logic slightly but shouldn't be a problem.

@github-actions

github-actions Bot commented Feb 6, 2024

Copy link
Copy Markdown
Contributor

Incompatible changes

slick

3 changes since 3.5.0-pre.26.b6e6680c

Code changes

Incompatibility Symbol Problem
Backward slick.compat.collection.package MissingClassProblem

class slick.compat.collection.package does not have a correspondent in current version
Backward slick.compat.collection.package$ MissingClassProblem

object slick.compat.collection.package does not have a correspondent in current version
Backward slick.compat.collection.package$JavaConverters$ MissingClassProblem

object slick.compat.collection.package#JavaConverters does not have a correspondent in current version

@mdedetrich
mdedetrich force-pushed the only-run-inliner-in-ci branch from 1a3d5c1 to 733e52d Compare February 6, 2024 00:54
@mdedetrich

mdedetrich commented Feb 6, 2024

Copy link
Copy Markdown
Contributor Author

@nafg Okay done, this version is actually cleaner/more correct since scala2InlineSettings is used in multiple places

@mdedetrich
mdedetrich force-pushed the only-run-inliner-in-ci branch from 733e52d to 042435d Compare February 6, 2024 00:56
@github-actions

github-actions Bot commented Feb 6, 2024

Copy link
Copy Markdown
Contributor

Incompatible changes

slick

3 changes since 3.5.0-pre.26.b6e6680c

Code changes

Incompatibility Symbol Problem
Backward slick.compat.collection.package MissingClassProblem

class slick.compat.collection.package does not have a correspondent in current version
Backward slick.compat.collection.package$ MissingClassProblem

object slick.compat.collection.package does not have a correspondent in current version
Backward slick.compat.collection.package$JavaConverters$ MissingClassProblem

object slick.compat.collection.package#JavaConverters does not have a correspondent in current version

1 similar comment
@github-actions

github-actions Bot commented Feb 6, 2024

Copy link
Copy Markdown
Contributor

Incompatible changes

slick

3 changes since 3.5.0-pre.26.b6e6680c

Code changes

Incompatibility Symbol Problem
Backward slick.compat.collection.package MissingClassProblem

class slick.compat.collection.package does not have a correspondent in current version
Backward slick.compat.collection.package$ MissingClassProblem

object slick.compat.collection.package does not have a correspondent in current version
Backward slick.compat.collection.package$JavaConverters$ MissingClassProblem

object slick.compat.collection.package#JavaConverters does not have a correspondent in current version

@mdedetrich
mdedetrich force-pushed the only-run-inliner-in-ci branch from 042435d to e8ff769 Compare February 6, 2024 01:49
nafg
nafg approved these changes Feb 6, 2024
View reviewed changes

@nafg nafg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks!

@github-actions

github-actions Bot commented Feb 6, 2024

Copy link
Copy Markdown
Contributor

Incompatible changes

slick

3 changes since 3.5.0-pre.26.b6e6680c

Code changes

Incompatibility Symbol Problem
Backward slick.compat.collection.package MissingClassProblem

class slick.compat.collection.package does not have a correspondent in current version
Backward slick.compat.collection.package$ MissingClassProblem

object slick.compat.collection.package does not have a correspondent in current version
Backward slick.compat.collection.package$JavaConverters$ MissingClassProblem

object slick.compat.collection.package#JavaConverters does not have a correspondent in current version

@mergify
mergify Bot merged commit 89f4f4b into slick:main Feb 6, 2024
@mdedetrich
mdedetrich deleted the only-run-inliner-in-ci branch February 6, 2024 02:03
@nafg nafg added the build Change to the build definition and dependencies label Feb 7, 2024
@nafg nafg added this to the 3.5.0 milestone Feb 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nafg nafg nafg approved these changes

Assignees

No one assigned

Labels

build Change to the build definition and dependencies

Projects

None yet

Milestone

3.5.0

Development

Successfully merging this pull request may close these issues.

2 participants

@mdedetrich @nafg