Bugzilla – Full Text Bug Listing
| Summary: | tycho-director-plugin supports different profile names for different environments | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | z_Archived | Reporter: | Meng Xin Zhu <kane.zhu> | ||||||||||||
| Component: | Tycho | Assignee: | Tobias Oberlies <t-oberlies> | ||||||||||||
| Status: | RESOLVED FIXED | QA Contact: | |||||||||||||
| Severity: | enhancement | ||||||||||||||
| Priority: | P3 | CC: | kane.mx | ||||||||||||
| Version: | unspecified | ||||||||||||||
| Target Milestone: | --- | ||||||||||||||
| Hardware: | All | ||||||||||||||
| OS: | All | ||||||||||||||
| Whiteboard: | |||||||||||||||
| Attachments: |
|
||||||||||||||
|
Description
Meng Xin Zhu
Created attachment 206251 [details]
patch
What do you think about this proposed patch? Thanks. Created attachment 206649 [details]
new version
fix NPE if no p2profile element in pom.xml.
The patch is good - I would just add a unit test for the logic that determines which profile wins. If you could do this this would be great. Otherwise, I'll just see when I come round to doing this. And another question: with the change, we now have the configuration parameter "profile" and "p2Profiles". How should they interact? (This is the question one needs to answer when documenting the parameters.) Currently, "profile" is used as fall-back, but we could also make them mutually exclusive, because <p2Profiles><p2Profile><profileName>the default<profileName></p2Profile></p2Profiles> achieve the same effect, right? Do you also have an example POM with the new parameter - just to see how the new parameter feels? (In reply to comment #4) > The patch is good - I would just add a unit test for the logic that determines > which profile wins. If you could do this this would be great. Otherwise, I'll > just see when I come round to doing this. I'll try it. > > And another question: with the change, we now have the configuration parameter > "profile" and "p2Profiles". How should they interact? (This is the question one > needs to answer when documenting the parameters.) Currently, "profile" is used > as fall-back, but we could also make them mutually exclusive, because > <p2Profiles><p2Profile><profileName>the > default<profileName></p2Profile></p2Profiles> achieve the same effect, right? Yes, they are same. If making them mutually exclusive, Tycho has to throw runtime exception if it can't decide what the user wants to(both definitions are found in pom.xml). Keeping "profile" definitely is for backward compatibility. The perfect solution is removing it. I think it might be possible, Tycho is still under incubation. ;) Created attachment 207401 [details]
an example pom for using different profile names for different materialized instances
@Meng: FYI, I'm currently working on a unit test. Tests are done. @Meng: I just realized that the new class P2Profile does not have a copyright header. What licence does apply for your patch? Who owns the copyright of the patch? (You could answer these questions through adding a header in the P2Profile class ;-) Created attachment 207456 [details]
new version of P2Profile.java with copyright header
(In reply to comment #9) > Created attachment 207456 [details] > new version of P2Profile.java with copyright header Cheers! Sorry for the IP-related hassle... Unless you have a strong opinion on this, I'd like to rename the parameter from "p2Profiles" to "profileNames". I'd like to make it clear (for non-p2 experts) that that parameter just affects a name and nothing else. Patches (with name change from P2Profile to ProfileName) submitted as 378d1b7. Tests submitted as e71bd18. (In reply to comment #10) > (In reply to comment #9) > > Created attachment 207456 [details] > > new version of P2Profile.java with copyright header > Cheers! Sorry for the IP-related hassle... > > Unless you have a strong opinion on this, I'd like to rename the parameter from > "p2Profiles" to "profileNames". I'd like to make it clear (for non-p2 experts) > that that parameter just affects a name and nothing else. "profileNames" is a nicer name. :) Thanks for quick response. It doesn't work. Looks like the comment style(/* @parameter */) after compiling can't be recognized as valid annotation in runtime. Created attachment 208141 [details]
patch
(In reply to comment #13) > It doesn't work. Looks like the comment style(/* @parameter */) after compiling > can't be recognized as valid annotation in runtime. Sorry, this was my fault. I introduced this bug when "reformatting" your three-line javadoc comment into a one-line javadoc comment. Except that it wasn't a javadoc comment any more. Thanks for finding this out. Fixed with d3f9da3. I should add an integration test to make sure this doesn't happen again... (In reply to comment #15) > I should add an integration test to make sure this doesn't happen again... Done with commit fbde176. @Meng: Thanks again for your contribution :-) |