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

Migrate p2 engine to agent properties #439

Merged
merged 1 commit into from
Feb 10, 2024

Conversation

laeubi
Copy link
Member

@laeubi laeubi commented Jan 25, 2024

Now that agent properties are possible we should use them in the engine, to not rely on only static OSGi context properties.

@laeubi laeubi requested a review from merks January 25, 2024 11:55
Copy link

github-actions bot commented Jan 25, 2024

Test Results

    9 files  ±0      9 suites  ±0   33m 5s ⏱️ +30s
2 183 tests ±0  2 179 ✅ ±0   4 💤 ±0  0 ❌ ±0 
6 639 runs  ±0  6 628 ✅ ±0  11 💤 ±0  0 ❌ ±0 

Results for commit 6397aad. ± Comparison against base commit fa81fb9.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@merks merks left a comment

Choose a reason for hiding this comment

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

It seems okay. But it causes one deprecation warning in Oomph for the phase set factory changes. I can't actually get rid of it by using the new method because I need to be able to run in older IDEs. I can't safely ignore the warning either because it's marked for removal. I could use reflection, but that is also ugly and error prone. So mostly I'm stuck with an ugly problem for which there are only ugly solutions.

There isn't a single iota of documentation to explain why I must use the new improved method. In fact, the deprecation doesn't even document which method should be used instead, nor even an indication when the removal will happen (no since). There's nothing the in Javadoc to suggest the agent could be null and why the agent is needed at all. If there is going to be disruptive change, the documentation should be better.

laeubi added a commit to laeubi/tycho that referenced this pull request Jan 30, 2024
laeubi added a commit to laeubi/tycho that referenced this pull request Jan 30, 2024
laeubi added a commit to eclipse-tycho/tycho that referenced this pull request Jan 30, 2024
github-actions bot pushed a commit to eclipse-tycho/tycho that referenced this pull request Jan 30, 2024
laeubi added a commit to eclipse-tycho/tycho that referenced this pull request Jan 31, 2024
@laeubi laeubi force-pushed the migrate_engine_to_agent_props branch from 08d7135 to 53aec3d Compare February 2, 2024 08:08
@laeubi laeubi requested a review from merks February 2, 2024 08:09
@laeubi laeubi force-pushed the migrate_engine_to_agent_props branch 4 times, most recently from c7bdbd3 to 3bca248 Compare February 8, 2024 09:38
@laeubi
Copy link
Member Author

laeubi commented Feb 8, 2024

@merks I was now able to rework this even without any deprecations!

Copy link
Contributor

@merks merks left a comment

Choose a reason for hiding this comment

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

This looks less disruptive.

@laeubi laeubi force-pushed the migrate_engine_to_agent_props branch from 3bca248 to b1d0dc1 Compare February 10, 2024 06:57
Now that agent properties are possible we should use them in the engine,
to not rely on only static OSGi context properties.
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