-
Notifications
You must be signed in to change notification settings - Fork 70
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
Log the args diff when re-running the smithy4s codegen #1559
Log the args diff when re-running the smithy4s codegen #1559
Conversation
29991ed
to
505006f
Compare
val core: Def.Initialize[ModuleID] = | ||
Def.setting("org.scalameta" %%% "munit" % munitVersion) | ||
val scalacheck: Def.Initialize[ModuleID] = | ||
Def.setting("org.scalameta" %%% "munit-scalacheck" % munitVersion) | ||
} | ||
object Munit extends MunitCross("0.7.29") | ||
object MunitMilestone extends MunitCross("1.0.0-M6") | ||
object MunitV1 extends MunitCross("1.0.0") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
may as well use 1.0.0 everywhere (or at the very least replace the milestone with it)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried that, but 1.0.0 is not published for native 0.4.x. This would involve upgrading native to 0.5.x and I'm not sure about cats-effect support there.
|
||
case Some(oldValue) => | ||
(serializeCodegenArgs(oldValue), serializeCodegenArgs(in)) match { | ||
case (Some(oldArgs), Some(newArgs)) if (oldArgs != newArgs) => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (oldArgs != newArgs)
string comparison is not enough, I'll switch to by-hash check
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah I'd rather you compared the json payloads than the json strings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's done now. We'll first compare JValue
s - if they differ we're able to produce the diff and log it. If arguments match structurally, hashes are checked to catch changes to the underlying files.
out of curiosity, what's the motivation for this change ? |
With the extensive use of smithy4s at $work, both my coworkers and myself we have noticed unexpected re-compilation of smithy sources. I'm not sure what caused it yet, I had no luck reproducing it with small projects, but it seems to happen in more complex sbt setups. A way to debug when and what exactly triggers smithy4s code generation would be a very helpful tool to proceed with the investigation. |
a521071
to
c3e4c63
Compare
409a896
to
87ae8d4
Compare
This PR introduces a change in Smithy4s sbt codegen plugin that would allow users to see what caused the codegen to re-execute. It is done by replacing
sbt.util.Tracked.inputChanged
with a similar implementation that keeps the entire codegen args serialized in cache, and when codegen needs to be rerun, plugin will print the diff of the values to debug log. The diff is calculated using munit-diff.When running
sbt
withset logLevel := Level.Debug
, user will notice a diff like this when codegen needs to rerun:PR Checklist (not all items are relevant to all PRs)