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

Initialize Plugin introduced keys to zero like values in globalSettings #121

Merged

Conversation

mdedetrich
Copy link
Collaborator

@mdedetrich mdedetrich commented Jan 25, 2024

Keys that are created/introduced by a plugin (i.e. OsgiKeys) should have their default values set in globalSettings with a zero like value otherwise it can cause issues with other sbt plugins/IDE's/editors.

@mdedetrich mdedetrich force-pushed the move-introduced-keys-into-global-settings branch from 63140e3 to 95e7f85 Compare January 25, 2024 01:07
@mdedetrich mdedetrich changed the title Move plugin introduced keys into globalSettings Initialize Plugin introduced keys to zero like values in globalSettings Jan 25, 2024
@eed3si9n
Copy link
Member

There's sbt plugin best practice called Provide default values in globalSettings, which states

If the default value of your settings or task does not transitively depend on a project-level settings (such as baseDirectory, compile, etc), define it in globalSettings.

What I should've added there is that such settings or tasks should not be specified in projectSettings. If you're eventually going to define the keys at project-level, then I don't know if makes much difference in defining a globally scoped one. For example,

requireCapability := Osgi.requireCapabilityTask(),

Given that Osgi.requireCapabilityTask() doesn't depend on any other settings, it would be most flexible if that definition remained at the global level, and unspecified anywhere else. That way the build user can override it either at ThisBuild level or at subproject level.

@mdedetrich mdedetrich force-pushed the move-introduced-keys-into-global-settings branch from 95e7f85 to 4c714a1 Compare January 25, 2024 05:41
@mdedetrich
Copy link
Collaborator Author

mdedetrich commented Jan 25, 2024

What I should've added there is that such settings or tasks should not be specified in projectSettings. If you're eventually going to define the keys at project-level, then I don't know if makes much difference in defining a globally scoped one. For example,

Yup these nuances/peculiarities I think I understand by now, I just made a mistake and didn't realize that Osgi.requireCapabilityTask() didn't depend on any other setting/task.

@eed3si9n I have just updated the PR with this fix (I also made Osgi.requireCapabilityTask a lazy val since it was a zero arg def before), let me know if I missed anything else.

Copy link
Member

@eed3si9n eed3si9n left a comment

Choose a reason for hiding this comment

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

Thanks!

@mdedetrich mdedetrich merged commit 77714f6 into sbt:main Jan 25, 2024
10 checks passed
@mdedetrich mdedetrich deleted the move-introduced-keys-into-global-settings branch January 25, 2024 20:27
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