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

MP 6 Issues PR #298

Merged
merged 11 commits into from
Dec 13, 2022
Merged

MP 6 Issues PR #298

merged 11 commits into from
Dec 13, 2022

Conversation

JanWesterkamp-iJUG
Copy link
Contributor

@Emily-Jiang, as discussed earlier, here is my PR regarding quick fix findings - the ones I found during my review and the ones I found during fixing the first findings.

Details of the changes could get sorted out by following my commit messages.

I reverted the changes regarding the very outdated AsciiDoc plugin versions getting aligned to the MicroProfile Parent ones - then the logo vanishes. Fixing that requires additional work (see PR there: eclipse/microprofile-parent#66), as fixing some of the formatting without risk too.
In the future we shoud think about reusing the Parent here.

@JanWesterkamp-iJUG
Copy link
Contributor Author

JanWesterkamp-iJUG commented Dec 13, 2022

@Emily-Jiang: I am preparing additonal fixes for the missing locale config (one position at least) and for the very outdated AsciiDoc configuration, using the proposed changes from the MicroProfile Parent as base (see eclipse/microprofile-parent#66).

Do you think it should be part of this PR or a separate one?
I think, it should be fixed for MP 6.0 too, as there is i.e. an alpha plugin version involved here at the moment. By the way, the change would also offer AsciiDoc Diagram support.

Copy link
Member

@Emily-Jiang Emily-Jiang left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks @JanWesterkamp-iJUG

@Emily-Jiang
Copy link
Member

@Emily-Jiang: I am preparing additonal fixes for the missing locale config (one position at least) and for the very outdated AsciiDoc configuration, using the proposed changes from the MicroProfile Parent as base (see eclipse/microprofile-parent#66).

Do you think it should be part of this PR or a separate one? I think, it should be fixed for MP 6.0 too, as there is i.e. an alpha plugin version involved here at the moment. By the way, the change would also offer AsciiDoc Dirgram support.

@JanWesterkamp-iJUG I suggest not to add more fixes before the ballot. We can do the subsequent fixes in the next one as the next release is around the corner. Thoughts?

@JanWesterkamp-iJUG
Copy link
Contributor Author

@Emily-Jiang: I am preparing additonal fixes for the missing locale config (one position at least) and for the very outdated AsciiDoc configuration, using the proposed changes from the MicroProfile Parent as base (see eclipse/microprofile-parent#66).
Do you think it should be part of this PR or a separate one? I think, it should be fixed for MP 6.0 too, as there is i.e. an alpha plugin version involved here at the moment. By the way, the change would also offer AsciiDoc Dirgram support.

@JanWesterkamp-iJUG I suggest not to add more fixes before the ballot. We can do the subsequent fixes in the next one as the next release is around the corner. Thoughts?

@Emily-Jiang I examined the old versions and there are exisitng vulnerabilities, that got fixed, see references here:

https://mvnrepository.com/artifact/org.asciidoctor/asciidoctor-maven-plugin/1.5.8 (4 CVEs)
vs.
https://mvnrepository.com/artifact/org.asciidoctor/asciidoctor-maven-plugin/2.2.2 (1 CVE)

Even in 2.2.2 some of it's preconfigured dependencies are updated in the POM - I can check the impact for that too, if you want me to.

So I strongly suggest we fix it, as we have a proven solution availabe yet, that solves three of four issues and it's a major update we can do only in a major update usually.

@Emily-Jiang
Copy link
Member

Emily-Jiang commented Dec 13, 2022

@JanWesterkamp-iJUG I thought this further. Since you only need to update the maven plugin version, go ahead to do a new commit on this PR.

@JanWesterkamp-iJUG
Copy link
Contributor Author

okay, @JanWesterkamp-iJUG please create a new PR with the new changes.

@Emily-Jiang I will do that, will you merge this one first?
Then I can rebase on it - that sould simplify the review and prevent merge conflicts.
In the worst case we can revert the new PR as a whole - so that would give some safety net.

@Emily-Jiang Emily-Jiang merged commit 9fec2d3 into eclipse:master Dec 13, 2022
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