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

KOGITO-10058: Migrate serverless-workflow-diagram-editor from private to KIE repos and libs #2169

Merged
merged 5 commits into from
Apr 23, 2024

Conversation

treblereel
Copy link
Contributor

@treblereel treblereel commented Feb 20, 2024

@ederign ederign requested review from caponetto and handreyrc and removed request for tiagobento February 21, 2024 15:48
@caponetto caponetto changed the title KOGITO-10058: Migrate serverless-workflow-diagram-editor from private… KOGITO-10058: Migrate serverless-workflow-diagram-editor from private to KIE repos and libs Feb 22, 2024
@LuboTerifaj
Copy link
Contributor

Hello @treblereel , could you please provide the details about the changes in related issue/jira? Thank you!

Copy link
Contributor

@LuboTerifaj LuboTerifaj left a comment

Choose a reason for hiding this comment

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

Hello @treblereel ,

org.treblereel.* dependencies were not removed from YARD editor.

Please see also my KOGITO-10058-comment

Other than that, all integration tests passed and I didn't find any issues.

Thank you!

@treblereel
Copy link
Contributor Author

treblereel commented Mar 14, 2024

Hello @treblereel ,

org.treblereel.* dependencies were not removed from YARD editor.

Please see also my KOGITO-10058-comment

Other than that, all integration tests passed and I didn't find any issues.

Thank you!

I think we should ask Eder and Mark to clarify this, but as far as I remember from our discussion, it's because of the Apache license: we can't move to Apache projects created by third parties.

Yard is @Rikkola 's project, so I think we can ask him to update the deps. I have informed him already that we published libs to jboss/sonatype so maybe we can just wait for him.

@LuboTerifaj
Copy link
Contributor

Hello @treblereel , KOGITO-10058 scope is to remove all dependencies from kie-tools repository. If you want to split the work with @Rikkola , we can do that, but we need to update the scope of the jira and report remaining work in a separate issue. Do you want to split the work?

@LuboTerifaj
Copy link
Contributor

One more note to my previous comment.

Alternatively, we can keep the scope of the jira the same, and link multiple PRs there, or use more commits in this PR and involve Toni here.

@Rikkola
Copy link
Contributor

Rikkola commented Mar 15, 2024

The model from YARD editor is going to move to incubator-kie-drools after 10.0.0. During the move I was planning to swap the treblereel yaml mapper dependency. If this needs to be done before 10.0.0, then I can swap it now.

Copy link
Contributor

@handreyrc handreyrc left a comment

Choose a reason for hiding this comment

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

Hey @treblereel,

It works well and I couldn't find any issues.

Thanks for your work here!

@LuboTerifaj
Copy link
Contributor

@Rikkola, @treblereel , how do you want to address the dependencies not removed from YARD editor? See my previous comments. Thanks!

@LuboTerifaj
Copy link
Contributor

@Rikkola I think it should be done before 10.0.0 release, so there are no third party libraries references in the repository.

@Rikkola
Copy link
Contributor

Rikkola commented Mar 20, 2024

@LuboTerifaj @treblereel I can do the change. There will be some manual testing needed, so I will be the fastest one to do it.

@treblereel
Copy link
Contributor Author

@Rikkola thank you, all green !

@Rikkola
Copy link
Contributor

Rikkola commented Mar 27, 2024

np

Copy link
Contributor

@LuboTerifaj LuboTerifaj left a comment

Choose a reason for hiding this comment

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

Please see my inline comment. Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

@treblereel , @Rikkola , can you please remove this file?

Copy link
Contributor

Choose a reason for hiding this comment

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

True, it should be removed.

Copy link
Contributor

@LuboTerifaj LuboTerifaj 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 updates @treblereel , @Rikkola . It looks good to me now.

@paulovmr paulovmr merged commit 7980222 into apache:main Apr 23, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants