Skip to content

Conversation

@ebarboni
Copy link
Contributor

No description provided.

@ebarboni ebarboni added Release process PRs (eg. versions, sync) that are part of the release process and can be ignored in release notes. do not merge Don't merge this PR, it is not ready or just demonstration purposes. labels Dec 10, 2024
@ebarboni
Copy link
Contributor Author

comment fixed to what version is in master (thanks @neilcsmith-net for assistance :D )

@ebarboni ebarboni removed the do not merge Don't merge this PR, it is not ready or just demonstration purposes. label Dec 10, 2024
Copy link
Member

Choose a reason for hiding this comment

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

this file is 14 MB large (!)

Copy link
Member

Choose a reason for hiding this comment

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

#7826 added 29 jars for a deploy aciton

Copy link
Member

Choose a reason for hiding this comment

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

Looks like they were wrongly attributed too - https://github.com/apache/netbeans/blob/master/enterprise/libs.fabric8/external/fabric8-2.17.2-license.txt#L4

I hope the license is correct!

@ebarboni
Copy link
Contributor Author

do you think we can merge, and let discussion on PR ?

@mbien
Copy link
Member

mbien commented Dec 11, 2024

IMO: I don't think it makes sense to maintain 14MB sig files over third party libs in our source code repo. The usefulness of sigs over dependencies is more of informational nature anyway since their API is not something we can influence. I don't have anything against them in principle - but a 14mb text file is a bit much in this particular case.

Whether the 29 new dependencies for a Kubernetes deploy action are justified or not I don't know, since I don't see any cost/benefit analysis or discussion anywhere which compares other options. But there are essentially three likely outcomes: a) PR gets reverted and we regen the sigs or b) cost/benefit is justified and we turn sig tests off for that set of dependencies. c) we merge it as is

I don't think we should merge this as is.

@neilcsmith-net
Copy link
Member

I would suggest removing that one file (org-netbeans-libs-fabric8.sig), amending and force pushing the commit, and getting this merged before anything else starts conflicting with it. It can always be added back later depending on where that conversation ends up.

@matthiasblaesing
Copy link
Contributor

The idea of @neilcsmith-net sounds like a good approach.

@mbien
Copy link
Member

mbien commented Dec 11, 2024

agreed. I thought about disabling the sig checks for the wrapper, but this could cause conflicts dependent on what is going to happen next. So removal of org-netbeans-libs-fabric8.sig from this PR without further changes sounds like the best approach to get this PR integrated without delays.

@ebarboni
Copy link
Contributor Author

sorry for delay sig file should be removed

@mbien
Copy link
Member

mbien commented Dec 12, 2024

diff stats with all one-line changes removed to make it easier to inspect:

 .../nbproject/org-netbeans-libs-jackson.sig   | 398 +++++++++++++++++-
 .../org-netbeans-modules-payara-common.sig    |  10 +-
 .../org-netbeans-modules-payara-micro.sig     |   9 +-
 .../org-netbeans-modules-payara-tooling.sig   |  18 +-
 .../org-netbeans-modules-websvc-restlib.sig   |   8 +-
 .../nbproject/org-netbeans-modules-gradle.sig |  26 +-
 .../nbproject/org-netbeans-api-lsp.sig        |   7 +-
 .../org-netbeans-modules-css-lib.sig          |   3 +-
 .../nbproject/org-netbeans-libs-git.sig       |   8 +-
 .../org-netbeans-modules-lsp-client.sig       |   5 +-
 .../org-netbeans-modules-parsing-indexing.sig |   3 +-
 ...rg-netbeans-modules-java-file-launcher.sig |   4 +-
 .../org-netbeans-modules-java-lexer.sig       |  10 +-
 .../org-netbeans-modules-java-source-base.sig |  54 ++-
 .../org-netbeans-modules-java-source.sig      |   7 +-
 .../org-netbeans-modules-java-sourceui.sig    |  13 +-
 .../org-netbeans-modules-maven-embedder.sig   | 295 +------------
 .../org-netbeans-modules-maven-indexer.sig    |  41 +-
 .../nbproject/org-netbeans-modules-maven.sig  |   5 +-
 .../org-netbeans-modules-refactoring-java.sig |  13 +-
 .../nbproject/org-netbeans-libs-asm.sig       |  11 +-
 .../nbproject/org-netbeans-libs-flatlaf.sig   |   5 +-
 .../org-netbeans-libs-javax-inject.sig        |  63 +++
 .../nbproject/org-netbeans-bootstrap.sig      |   4 +-

@ebarboni ebarboni merged commit 7d38f33 into apache:master Dec 12, 2024
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Release process PRs (eg. versions, sync) that are part of the release process and can be ignored in release notes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants