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

Solved some implementation and design smells #5023

Closed
wants to merge 35 commits into from

Conversation

gunjanVazirani
Copy link

What's the purpose of this PR

Have solved 3 design smells and 3 implementation smells as a part of our assignment

Which issue(s) this PR fixes:

Fixes # It resolves design smells and implementation smells in different sections of the code.

…service/AdditionalUserInfoEnrichService.java
…service/AdditionalUserInfoEnrichServiceImpl.java
…rk/apollo/audit/aop/ApolloAuditSpanAspect.java
@dosubot dosubot bot added the size:XXL This PR changes 1000+ lines, ignoring generated files. label Nov 27, 2023
Copy link

CLA Assistant Lite bot:
Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.


I have read the CLA Document and I hereby sign the CLA


You can retrigger this bot by commenting recheck in this Pull Request

fieldName = ((ApolloAuditLogDataInfluenceTableField) annotations[j]).fieldName();
}
}
String[] strings= entityAndField(annotations);
Copy link
Member

Choose a reason for hiding this comment

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

Would you please help to explain what the code smell was?


protected String dataChangeCreatedBy;

protected String namespaceName;
Copy link
Member

Choose a reason for hiding this comment

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

Why is namespaceName added here?

import org.springframework.web.context.request.ServletRequestAttributes;

@Aspect
public class ApolloAuditSpanAspect {
Copy link
Member

Choose a reason for hiding this comment

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

What's this change about?

@@ -109,17 +109,17 @@ public void testWhenHasReleaseMsgAndHasRepeatMsg() throws Exception {
assertEquals(3, latestReleaseMsg.getId());
assertEquals(anotherMsgContent, latestReleaseMsg.getMessage());

List<ReleaseMessage> latestReleaseMsgGroupByMsgContent =
List<ReleaseMessage> groupedLatestMsgs =
Copy link
Member

Choose a reason for hiding this comment

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

Why was the variable renamed?

<T> void enrichAdditionalUserInfo(List<? extends T> list,
Function<? super T, ? extends UserInfoEnrichedAdapter> mapper);

<T> List<UserInfoEnrichedAdapter> adapt(List<? extends T> dtoList,
Copy link
Member

Choose a reason for hiding this comment

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

Why adding this methond to the interface?

Copy link

stale bot commented Dec 28, 2023

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 14 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@stale stale bot added the stale label Dec 28, 2023
Copy link

stale bot commented Jan 11, 2024

This pull request has been automatically closed because it has not had activity in the last 14 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@stale stale bot closed this Jan 11, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Jan 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
size:XXL This PR changes 1000+ lines, ignoring generated files. stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants