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

support older mill 0.10.x 0.11.y patch versions #166

Conversation

james-s-w-clark
Copy link

@james-s-w-clark james-s-w-clark commented Nov 6, 2023

@lefou could you sanity check this is a good suggestion? 😅 I don't fully understand, but trying to copy the pattern I'm seeing. I've been looking for possible mill issues in the 0.11.1 -> 0.11.2 bump related to the --disable-callgraph-validation Mill issue.

Copy link
Contributor

@lefou lefou 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 to me.

@james-s-w-clark
Copy link
Author

@joan38 WDYT?

Also, should this be updated to some mill 0.11.x version? https://github.com/joan38/mill-scalafix/blob/main/.mill-version

@joan38
Copy link
Owner

joan38 commented Nov 6, 2023

@IdiosApps looks good.
For the mill version of this project we are stuck on 0.10 versions until BSP is repaired in 0.11

@joan38
Copy link
Owner

joan38 commented Nov 7, 2023

@IdiosApps Could you fix the compilation?

Also could you explain the motivation for supporting older patch versions?

@james-s-w-clark
Copy link
Author

@IdiosApps Could you fix the compilation?

Also could you explain the motivation for supporting older patch versions?

I can try and fix it another day, sure.

Like I said, I don't fully understand, but trying to copy the pattern I'm seeing 😅 @lefou could you explain it a bit?

@joan38
Copy link
Owner

joan38 commented Nov 7, 2023

I understand mill is not forward compatible and therefore plugins need to be depending on older versions of mill than the end project is using. But why can't the end project upgrade the patch version? That is what I'm trying to understand.

Also the example you linked seem to depend on 0.10.12

@lefou
Copy link
Contributor

lefou commented Nov 7, 2023

There are always reasons to not use the latest versions. (The OP just provided an example: using an older version because a newer minor version introduces an issue and nobody else saw it so far.) If it does not hurt, why should one want to limit the compatibility of a plugin to only the latest version?

@lefou
Copy link
Contributor

lefou commented Nov 7, 2023

Btw, compatibility-wise, we're speaking about minor-versions here, as we are in early-semver which effectively means 0.major.minor.

@lefou
Copy link
Contributor

lefou commented Nov 7, 2023

And regarding this PR, we can't lower the version back to 0.10.0, as the semanticDbData target was only introduced in Mill 0.10.6, I think.

build.sc Outdated Show resolved Hide resolved
@lolgab
Copy link
Collaborator

lolgab commented Nov 8, 2023

I think Mill 0.10 is now a "legacy" version of Mill, and I would be okay supporting only its latest version (0.10.12). Bumping to the latest patch version it's a no-brainer since they are backward compatible.
Also Mill 0.11.1 was released as a bug-fix release for 0.11.0 so I don't see point on supporting 0.11.0

Co-authored-by: Tobias Roeser <le.petit.fou@web.de>
@lefou
Copy link
Contributor

lefou commented Nov 8, 2023

I think Mill 0.10 is now a "legacy" version of Mill, and I would be okay supporting only its latest version (0.10.12). Bumping to the latest patch version it's a no-brainer since they are backward compatible. Also Mill 0.11.1 was released as a bug-fix release for 0.11.0 so I don't see point on supporting 0.11.0

This sounds reasonable. Yet, I'd like to ask why one should want to explicitly unsupport a certain version without any needs? We're talking about API here, not implementation. If the user runs with a newer version, e.g. to get all the upstream fixes and improvements, the plugin will use that newer version too.

From a users perspective, there is no easy way to detect, whether a plugin was build against a compatible API, besides the binary platform version. When analyzing build issues, this kind of incompatibility is less obvious and may lead to obscure error messages. And, tbh, this is against any best practice in plugin development for any kind of system I ever worked with.

@lefou
Copy link
Contributor

lefou commented Nov 16, 2023

I just want to bring to your awareness, that there is a mill-dependency-check plugin that depends on this plugin, so using a proper (low) API baseline is even more important.

I also found the following line in its build.sc, which could be a good indicator for the kind of issues we might produce if we do not care about compatibility.

https://github.com/Eleven19/mill-dependency-check/blob/fc4c8222faaf7b072c43a1a73b75102f04214c86/build.sc#L17

// TODO Should probably drop this to 0.10.0, but when I did a bunch of stuff breaks
val millVersion = "0.10.3"

@lefou
Copy link
Contributor

lefou commented Jan 7, 2024

Mostly superseded by #172, which raised the lowest supported Mill version to 0.10.15. Maybe, the scala-steward:off comment is worth to add.

@joan38
Copy link
Owner

joan38 commented Jan 7, 2024

Good point !

@joan38
Copy link
Owner

joan38 commented May 16, 2024

Closing this one. Reopen if needed

@joan38 joan38 closed this May 16, 2024
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