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

Update toolchain to use Tycho 4.0.x and Maven 3.9.x #1458 #1459

Conversation

claesrosell
Copy link
Contributor

No description provided.

@claesrosell
Copy link
Contributor Author

@speckyspooky What do you think about the removal of the design dependency? It looks like it was introduced accidentally.
@wimjongman Does the Changes to the build.look okey?

Copy link
Contributor

@wimjongman wimjongman left a comment

Choose a reason for hiding this comment

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

Looks good to me, Claes. @merks also OK?

@merks
Copy link
Contributor

merks commented Oct 27, 2023

If the build works and the tests ran/passed it must be okay.

@claesrosell claesrosell linked an issue Oct 27, 2023 that may be closed by this pull request
@claesrosell
Copy link
Contributor Author

I see that we use some deprecated actions in our CI workflow. We should probably update those as well. I will open a separate issue for that.

@claesrosell claesrosell merged commit 30df22d into eclipse-birt:master Oct 27, 2023
3 checks passed
@speckyspooky
Copy link
Contributor

Normally I would agree Ed, but we have some changes which will definitely not tested with the default tests.
So it good be that in a very specific case it was used but then we have to validate it and optimize the code there.

@merks
Copy link
Contributor

merks commented Oct 28, 2023

@speckyspooky

There are three changes here. The ci.yml changes affects only the github actions. The MANIFEST.MF change affects only dependencies so I believe that if the build runs, then it's not harmful. The pom.xml change affects which version of Maven/Tycho are used; using newer ones will be necessary sooner or later and it appears that sooner already arrived with the build problems that @claesrosell encountered that drove that change.

Is it one of these three specific changes that concerns you? I would have expected that it's the lack of tests for the runtimes and the lack of tests for the viewers that would be our primary cause for concern, not the changes in this PR...

@speckyspooky
Copy link
Contributor

Yes, I'm fine with the Tycho update!
The only thing is realy the MANIFEST.MF change. My brain means that I saw such kind of change in the past but I cannot say in which constellation and if the modification & dependency used at the end.
I think the build will work and BIRT wil be runnable. And if I was able to execute my tests and my reports are running then we are fine with all.

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.

Update toolchain to use Tycho 4.0.x and Maven 3.9.x
4 participants