-
Notifications
You must be signed in to change notification settings - Fork 11
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
[fix]: SDE backend : PCF schema update with security issue fixed #74
[fix]: SDE backend : PCF schema update with security issue fixed #74
Conversation
Hi @adityagajbhiye9 and @sachinargade123 thanks for splitting your huge PR. Just to remind you as part of the System Team i can not validate the content and the functionality against it. I can only rely on your testing strategy and your knowledge how your product works. Also as followed our TRG´s you can also have a quick info what to ensure in your product. https://eclipse-tractusx.github.io/docs/release I would also recommend the idea to have at least one committer in your product team. So please checkout also our guideline how to become a committer in our Eclipse project. https://eclipse-tractusx.github.io/docs/oss/contributor-committer#committer As a general reminder following conventional commits https://www.conventionalcommits.org/en/v1.0.0/ it is not recommended to use fixes and features in one PR could be missleading and could break your features. Unless for upcomming release it is nessesary i know ... but keep in mind that you have to work against Eclipse Tractus X GitHub organisation not Catena-X GitHub Orga... (this is the reason to have a committer in your round) |
Best Idea is that someone who can verify the content updates the PR´s and if no committer in your team is available, we can merge it but we have to trust your changes, so please keep aware of that. |
can you please add to the description why you want to downgrade Java 18 -> 17? |
Apart from that the PR LGTM. Nevertheless it can't be merged until @adityagajbhiye9 has signed the ECA. (automatic check will fail) |
.../org/eclipse/tractusx/sde/common/submodel/executor/create/steps/impl/RecordProcessUtils.java
Show resolved
Hide resolved
LGTM |
thanks for commenting the issue with your statement, but i would kindly ask if you can review the PR with an approval in the code section and then there is a "approve" option. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see my comment below. Rest LGTM
Description
Fixed
Pre-review checks
Please ensure to do as many of the following checks as possible, before asking for committer review: