-
Notifications
You must be signed in to change notification settings - Fork 5
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
Add pom to build updatesite #18
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything works perfectly :) See minor comments inline.
Realistically, this is good to merge IMO (I could address the comments myself).
Amazing work @ingomohr !
releng-updatesite/category.xml
Outdated
<feature id="aobuchow.themes.moderndark.feature"> | ||
<category name="aobuchow.themes.moderndark.update" /> | ||
</feature> | ||
<category-def name="aobuchow.themes.moderndark.update" label="Modern Dark Theme update site"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
renamed
<modules> | ||
<module>aobuchow.themes.moderndark</module> | ||
<module>aobuchow.themes.moderndark.feature</module> | ||
<module>releng-updatesite</module> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be worth including the product as a module to build, as I recently found out you can use a .product to configure Eclipse Preferences (currently, all preferences are set in CSS).
If we decide to include the product, it's feature needs to be updated to aobuchow.themes.moderndark.feature
However, I think this could be for a future PR :) It's not a requirement for this to get merged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added product
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The built product won't start on my machine (MacOS Catalina). Did it start on yours before?
I checked it against https://github.com/vogellacompany/rcpdemo/tree/master/com.vogella.tasks.product but that one also doesn't after I built it w/ Maven.
Maybe I'm missing something...?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No worries, I'll look into it or make a bug about it at least.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For testing I recommend to use https://github.com/vogellacompany/tycho-example
Great resource :D @vogella
I haven't tested this but it looks sane to me :)
The Export folder can definitely be removed. I might want to keep the product folder.
This is a very interesting idea, I'll have to look into it.
It was a challenge for me to find something that could be improved in this PR ;) |
2a9deee
to
b210602
Compare
Looks good to me! Merging. Amazing first PR @ingomohr, very grateful for the work you did here :) |
pom.xml
to every projectyml
script to have Github automatically build the updatesite on every push to the Git repo.Local Test
I installed the feature from the locally built updatesite. The theme looks as before to me. Maybe you could doublecheck.
Thoughts
I think the product and the Export folder could be removed (later).
Maybe there's a way to establish all versions as Composite Updatesite via Github Actions (I didn't yet check that out, though),
Note I'm looking forward to any review feedback. :) Maybe you find something to be improved, as well. :)