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

DEMO_BRANCH #35

Closed
wants to merge 187 commits into from
Closed

Conversation

Diogo12246
Copy link
Contributor

@Diogo12246 Diogo12246 commented Dec 12, 2023

Description

Pre-review checks

Please ensure to do as many of the following checks as possible, before asking for committer review:

@SebastianBezold
Copy link
Contributor

SebastianBezold commented Dec 13, 2023

Hey guys, sorry to say, but what is going on here.
See my comments on #33.

I was asking for small PRs and proper descriptions.
Now you opened a new PR, with exactly the same title, still no description and exactly the same branch and commit log

Please take my comments seriously and improve your work.
We won't accept these kind of contributions

EDIT: Here additional info on lange PRs and the need to engage with the IP team: https://www.eclipse.org/projects/handbook/#ip-project-content

@Siegfriedk
Copy link
Contributor

@Diogo12246 please be aware that what @SebastianBezold wrote is exactly what i would have written.

In OpenSource, this PR is basically a slap in the face for anyone who needs to review 187 commits.

Do you need onboarding support on how to work in OpenSource?

@Diogo12246
Copy link
Contributor Author

@SebastianBezold

Hello Sebastian, my understanding (incorrect one it seems) was that you wished for us to open a PR with no additional changes or mistaken commits. Sorry for the misunderstanding.

Regarding the branch itself @Siegfriedk

It is a compilation of all the features done to present at the tech demo of DCMFOSS all in one branch.
Having to go back and break this PR into a lot of chunks and prs will be time consuming on schedule.
Is there any way we could merge this PR without breaking off to multiple chunks?
What validations are required? we updated the dependency with the dependency tool. I see there is a error still in front end we will fix it and add it here.

Thank you in advance

@Siegfriedk
Copy link
Contributor

@Diogo12246 you are working under the Eclipse Foundation. Its your duty to read up on how to work in this organization and there is a eclipse handbook.

Mixing and building a huge PR which no one can really review is just a no go.

At least from my point of view i don't have time until in the middle of january for a 'deep review' with you together or something like this.

Its still your best bet to split it up properly and do it right.

@Diogo12246
Copy link
Contributor Author

@Siegfriedk

Alright!
Thank you for the clear explanation
We will close this one and discuss with the team to break this into multiple chunks and submit more PRs.
Expect more PRs to emerge, and of course, please do warn if more and smaller are needed so we can in the team approach this correctly.

Thank you kindly and sorry for any misunderstanding.
Diogo.

@Diogo12246 Diogo12246 closed this Dec 14, 2023
SebastianBezold pushed a commit that referenced this pull request Jan 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants