search

eclipse-cdt / cdt-lsp

Eclipse CDT™ LSP Extensions for CDT
Eclipse Public License 2.0
23 stars 11 forks source link

[276] Refactor API #277

Closed ghentschke closed 4 months ago

ghentschke commented 4 months ago

to provide a stable API with v2.0.0

ghentschke commented 4 months ago

I fixed the unit tests. Please review @ruspl-afed

ruspl-afed commented 4 months ago

@ghentschke , what is your plan regarding this one? My suggestion is to merge it "as is" and then have another iteration of API refinement.

ghentschke commented 4 months ago

what is your plan regarding this one? My suggestion is to merge it "as is" and then have another iteration of API refinement

I am currently working on another issue but I wanted to catch up some of your comments which can be fixed fast. I think I can do that today.

ghentschke commented 4 months ago

I would at least fix the build error in the test

ghentschke commented 4 months ago

@ruspl-afed or @jonahgraham Do have na idea why the master cannot be build? It claims that the maven version must be 3.9.0.

[ERROR] Failed to execute goal org.eclipse.tycho:tycho-source-plugin:4.0.6:plugin-source (plugin-source) on project org.eclipse.cdt.lsp.root: The plugin org.eclipse.tycho:tycho-source-plugin:4.0.6 requires Maven version 3.9.0 -> [Help 1]

But the maven version has already been updated here: https://github.com/eclipse-cdt/cdt-lsp/blob/e45bfa79a6107a64ebc08b820a7166bb4c1471db/.github/workflows/tycho-build.yaml#L19

akurtakov commented 4 months ago

This is controlled by https://github.com/eclipse-cdt/cdt-lsp/blob/master/Jenkinsfile and not the GHA file. Unfortunately, I have no idea how your docker image is defined to be able to help you more. In not that custom environments smth like https://github.com/eclipse-platform/eclipse.platform.ui/blob/04dd9a1bf255b0af610082a38d79ccd4fa861084/Jenkinsfile#L11 is enough.

akurtakov commented 4 months ago

Probably changing the mvn executable used to the one that cdt uses https://github.com/eclipse-cdt/cdt/blob/95fe4d87010b1446741bedd8818bea3252ebc8be/Jenkinsfile#L40C23-L40C61 should do the trick.

jonahgraham commented 4 months ago

Sorry - @ghentschke I forgot that I had updated only main CDT Jenkinsfile and not the cdt-lsp one. PR incoming.

ghentschke commented 4 months ago

@akurtakov Thank you for the hints!