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

feat(rest): Add size parameter to clearing request. #2642

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

akshitjoshii
Copy link
Contributor

Add a new field size to the clearing request which determines the size of the clearing request based on the number of open releases in the project for which the CR is created.

Size based on the table below :
1- 20 = Very Small
21- 50 = Small
50-75 = Medium
75-150 = Large
150 and above = Very large

The CR size should be dynamic. if release are uploaded after CR creation, it should reflect in the CR Size.

image

@akshitjoshii akshitjoshii added needs code review needs general test This is general testing, meaning that there is no org specific issue to check for labels Oct 16, 2024
@akshitjoshii akshitjoshii force-pushed the feat/add-size-field-cr-endpoints branch from 72fc0f3 to 7a056d3 Compare October 16, 2024 10:45
@sameed20
Copy link
Contributor

Testing this PR.

@akshitjoshii akshitjoshii force-pushed the feat/add-size-field-cr-endpoints branch 3 times, most recently from 8103c82 to 1916a1b Compare October 23, 2024 06:24
@sameed20
Copy link
Contributor

@akshitjoshii When increasing the number of releases in a project, the size of the CR should increase accordingly. However, I observed that when fetching a CR by ID from an endpoint, the size parameter in the response does not reflect the updated size value, even though the size field in the database updates as expected.

@akshitjoshii akshitjoshii force-pushed the feat/add-size-field-cr-endpoints branch 2 times, most recently from 5010d20 to 7d441e0 Compare October 30, 2024 06:48
@akshitjoshii
Copy link
Contributor Author

@sameed20 , made the changes. Pls check

@sameed20
Copy link
Contributor

sameed20 commented Nov 4, 2024

Test successful.

@akshitjoshii akshitjoshii removed the needs general test This is general testing, meaning that there is no org specific issue to check for label Nov 4, 2024
@akshitjoshii akshitjoshii force-pushed the feat/add-size-field-cr-endpoints branch 2 times, most recently from 5a71bae to 76dbdc3 Compare November 5, 2024 09:28
@akshitjoshii akshitjoshii force-pushed the feat/add-size-field-cr-endpoints branch from 76dbdc3 to 8c8f4cb Compare November 11, 2024 09:33
GMishx
GMishx previously approved these changes Nov 13, 2024
Copy link
Member

@GMishx GMishx left a comment

Choose a reason for hiding this comment

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

Changes looks good.

@GMishx GMishx added ready ready to merge and removed needs code review labels Nov 13, 2024
@GMishx
Copy link
Member

GMishx commented Nov 13, 2024

@afsahsyeda can you please review the changes for auto merge of this branch?

Copy link
Contributor

@heliocastro heliocastro left a comment

Choose a reason for hiding this comment

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

Approved but with some remarks with code style.

import org.eclipse.sw360.datahandler.thrift.RemoveModeratorRequestStatus;
import org.eclipse.sw360.datahandler.thrift.RequestStatus;
import org.eclipse.sw360.datahandler.thrift.SW360Exception;
import org.eclipse.sw360.datahandler.thrift.*;
Copy link
Contributor

Choose a reason for hiding this comment

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

In the future, remember that best practices is no do global imports.

@heliocastro heliocastro added the has merge conflicts The PR has merge conflicts label Nov 19, 2024
@heliocastro
Copy link
Contributor

@akshitjoshii Please resolve the merge conflict and if possible revert the glob on imports.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has merge conflicts The PR has merge conflicts ready ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants