search

eclipse-equinox / equinox.bundles

Eclipse Public License 2.0
8 stars 16 forks source link

Publish the a "BundleStateScope" as an IScopeContext service (factory) #25

Closed laeubi closed 2 years ago

laeubi commented 2 years ago

Fix https://github.com/eclipse-equinox/equinox.bundles/issues/24

laeubi commented 2 years ago

@tjwatson can you review this?

laeubi commented 2 years ago

FYI @vogella

vogella commented 2 years ago

Can we get a better commit messages? "Publish the a "BundleStateScope" as an IScopeContext service (factory)" is a great header but the commit message should contain a short description of the purpose of the change (focusing on the why). I assume most of this can be copied from the isssue.

laeubi commented 2 years ago

If you like you can suggest a message to include, I found commit messages not very well suited to describe whats going on and have already described in the javadoc and issue the possible uses. Probably a N&N would be good as well if this is merged.

tjwatson commented 2 years ago

If you like you can suggest a message to include, I found commit messages not very well suited to describe whats going on and have already described in the javadoc and issue the possible uses. Probably a N&N would be good as well if this is merged.

The commit message is really useful for when browser the blame values in the IDE or otherwise for individual lines of code. So any additional hints there help the future contributors when they are looking at changes that may not be directly where you provided the javadoc.

laeubi commented 2 years ago

by the way if you thing enabling declarative service for org.eclipse.equinox.preferences is not desired I can think of using a plain service registration as it is not that complex to do it without DS here.

laeubi commented 2 years ago

I updated the commit message just in case someone needs to "blame" the commit (I really like the EGit term "Show revision Information" much better...).

tjwatson commented 2 years ago

by the way if you thing enabling declarative service for org.eclipse.equinox.preferences is not desired I can think of using a plain service registration as it is not that complex to do it without DS here.

That may be for the best, but not because of the component.xml confusion. Up until now the preferences bundle has been able to run without a DS implementation.

laeubi commented 2 years ago

by the way if you thing enabling declarative service for org.eclipse.equinox.preferences is not desired I can think of using a plain service registration as it is not that complex to do it without DS here.

That may be for the best, but not because of the component.xml confusion. Up until now the preferences bundle has been able to run without a DS implementation.

Done.

laeubi commented 2 years ago

LGTM

Would you kindly merge this then?

tjwatson commented 2 years ago

Would you kindly merge this then?

I mistakenly thought your committer election was already complete. Should be done this week I think. I did merge.