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

[JENKINS-72981] Inline EZMorph repository #8

Merged
merged 4 commits into from
Apr 24, 2024

Conversation

BobDu
Copy link
Member

@BobDu BobDu commented Apr 8, 2024

json-lib has a exclusive dependency libary ezmorph

Currently, we not have a fork for ezmorph. However, the refactoring work will inevitably involve modifications to the content of ezmorph. Considering the singularity of dependencies. It is also unnecessary to separately split ezmorph into an independent repository and jar package.

So, attempting merge ezmorph into this repository.

I make a ezmorph readonly git mirror from origin cvs for reference, and copy all java code into this repo.

https://github.com/BobDu/ezmorph

@basil

Testing done

Submitter checklist

Preview Give feedback

@BobDu BobDu changed the title merge ezmorph repository to this repository [JENKINS-72981] step1: merge ezmorph repository to this repository Apr 8, 2024
@BobDu BobDu marked this pull request as ready for review April 8, 2024 03:14
@KalleOlaviNiemitalo
Copy link

Does the ezmorph source tree contain a NOTICE file as referenced by clause 4.d. of the Apache-2.0 license? Or any other legal notices that would have to be copied with the Java files.

@BobDu
Copy link
Member Author

BobDu commented Apr 8, 2024

Does the ezmorph source tree contain a NOTICE file as referenced by clause 4.d. of the Apache-2.0 license? Or any other legal notices that would have to be copied with the Java files.

I make a readonly git repository replica covert from origin cvs repo.

https://github.com/BobDu/ezmorph

In the original repository, there are no additional notice files.

Signed-off-by: Bob Du <i@bobdu.cc>
@BobDu BobDu force-pushed the merge-ezmorph-repo branch 2 times, most recently from e056960 to 1cf9067 Compare April 12, 2024 11:08
@BobDu BobDu changed the title [JENKINS-72981] step1: merge ezmorph repository to this repository [JENKINS-72981] Merge ezmorph repository to this repository Apr 12, 2024
Copy link
Member

@basil basil left a comment

Choose a reason for hiding this comment

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

Very nice! Can you also copy over the package.html files from the original repository? We have the package.html files for JSON-lib in this repository, so we should keep the package.html files from EZMorph to be consistent.

What's the reason for removing the unused files in commit 1cf9067, and how do you know they are unused? In theory the only way to prove that a plugin isn't consuming them would be using https://github.com/jenkins-infra/usage-in-plugins. And this also dilutes the semantics of this PR, since it's now doing two things, a code import and a code cleanup. I think I would rather let this PR be a simple import and defer the code cleanup to another PR, unless there was a particular reason to do so here.

@BobDu
Copy link
Member Author

BobDu commented Apr 13, 2024

so we should keep the package.html files from EZMorph to be consistent.

Origin package.html file has some html tag syntax error cause CI failed, and there doesn't seem to be any meaningful information. So, I rough delete it.
But I agree with your point of view, keep copying the original code as much as possible in this PR except for fixing CI errors.

I think I would rather let this PR be a simple import and defer the code cleanup to another PR

+1, I will revert this commit in this PR

Signed-off-by: Bob Du <i@bobdu.cc>
@BobDu BobDu force-pushed the merge-ezmorph-repo branch from 1cf9067 to c799d68 Compare April 13, 2024 02:47
@BobDu
Copy link
Member Author

BobDu commented Apr 13, 2024

Origin package.html file has some html tag syntax error cause CI failed

image

Sorry, I made a mistake. Looks like a hint but does not cause failure.
So let’s not modify it yet, in this PR.

Copy link
Member

@basil basil left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

I have confirmed that these sources exactly match the contents of the 1.0.6 sources JAR on Maven Central and the CVS repository checked out via:

cvs -z3 -d:pserver:anonymous@ezmorph.cvs.sourceforge.net:/cvsroot/ezmorph co -r REL_1_0_6 -P ezmorph

There was a minor textual merge conflict in the log4j properties file, which I resolved in favor of EZMorph to ensure that tests have proper debug logs.

@basil basil added the internal label Apr 24, 2024
@basil basil merged commit 893eab2 into jenkinsci:master Apr 24, 2024
9 of 11 checks passed
@basil basil changed the title [JENKINS-72981] Merge ezmorph repository to this repository [JENKINS-72981] Inline EZMorph repository Apr 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants