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

JS-261 Update easy deps #4825

Merged
merged 3 commits into from
Sep 19, 2024
Merged

JS-261 Update easy deps #4825

merged 3 commits into from
Sep 19, 2024

Conversation

zglicz
Copy link
Contributor

@zglicz zglicz commented Sep 17, 2024

Use ncu -i --format group and for now, only bump the easy ones.

@zglicz zglicz marked this pull request as draft September 17, 2024 14:43
Comment on lines 365 to 376
<minsize>80000000</minsize>
<maxsize>87500000</maxsize>
<maxsize>90000000</maxsize>
<files>
<file>${project.build.directory}/${project.build.finalName}-multi.jar</file>
</files>
</requireFilesSize>
<requireFilesSize>
<minsize>30000000</minsize>
<maxsize>50000000</maxsize>
<maxsize>51000000</maxsize>
<files>
<file>${project.build.directory}/${project.build.finalName}-linux-x64.jar</file>
<file>${project.build.directory}/${project.build.finalName}-win-x64.jar</file>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updating the sizes, as the jar is just slightly above the limits, so it seems like only a slight increase.

[ERROR] Rule 0: org.apache.maven.enforcer.rules.files.RequireFilesSize failed with message: [ERROR] /tmp/cirrus-ci-build/sonar-plugin/sonar-javascript-plugin/target/sonar-javascript-plugin-10.15.0-SNAPSHOT-multi.jar size (87528440) too large. Max. is 87500000/tmp/cirrus-ci-build/sonar-plugin/sonar-javascript-plugin/target/sonar-javascript-plugin-10.15.0-SNAPSHOT-multi.jar
[ERROR] Rule 1: org.apache.maven.enforcer.rules.files.RequireFilesSize failed with message: [ERROR] /tmp/cirrus-ci-build/sonar-plugin/sonar-javascript-plugin/target/sonar-javascript-plugin-10.15.0-SNAPSHOT-linux-x64.jar size (50579040) too large. Max. is 50000000/tmp/cirrus-ci-build/sonar-plugin/sonar-javascript-plugin/target/sonar-javascript-plugin-10.15.0-SNAPSHOT-linux-x64.jar [ERROR]
[ERROR] /tmp/cirrus-ci-build/sonar-plugin/sonar-javascript-plugin/target/sonar-javascript-plugin-10.15.0-SNAPSHOT.jar size (27659105) too large. Max. is 27500000/tmp/cirrus-ci-build/sonar-plugin/sonar-javascript-plugin/target/sonar-javascript-plugin-10.15.0-SNAPSHOT.jar

@zglicz zglicz marked this pull request as ready for review September 18, 2024 08:57
@zglicz zglicz requested a review from a team September 18, 2024 11:57
Copy link
Contributor

@vdiez vdiez left a comment

Choose a reason for hiding this comment

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

Please sync versions with packages/jsts/src/rules/package.json

Comment on lines +321 to +328
onBackreferenceEnter: reference => {
const shouldSaveReference = isAmbiguousGroup(reference)
? reference.resolved.filter(capturingGroup => capturingGroup.name).length > 0
: reference.resolved.name !== null;
if (shouldSaveReference) {
backreferences.push(reference);
}
},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updating this logic, as the typing changed in version 4.11.0 - eslint-community/regexpp@fb20f68

Comment on lines +313 to +315
function isAmbiguousGroup(reference: Backreference): reference is AmbiguousBackreference {
return reference.ambiguous;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding this as, just checking in the line below for: reference.ambigous ? "treat as ambiguous" : "treat as unambiguous" did not pass jest tests.... Looked a bit with @ericmorand-sonarsource , but failed to figure it out.

@zglicz zglicz requested a review from vdiez September 18, 2024 13:53
Copy link
Contributor

@vdiez vdiez left a comment

Choose a reason for hiding this comment

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

just small comment

@@ -326,7 +342,11 @@ function makeGroupKnowledge(
index: number,
): GroupKnowledge {
const name = node.name!;
const used = backreferences.some(backreference => backreference.resolved === node);
const used = backreferences.some(backreference =>
backreference.ambiguous
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't we use same helper here?

Copy link
Contributor Author

@zglicz zglicz left a comment

Choose a reason for hiding this comment

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

All of the ruling changes are accounted for.

@@ -1,6 +1,5 @@
{
"ant-design:components/upload/__tests__/upload.test.js": [
37,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This stopped showing up due to an update in eslint-react-plugin in v7.36 - commit jsx-eslint/eslint-plugin-react#3807

Comment on lines -9 to -18
28,
38
],
"console:src/components/Help/Help.tsx": [
28,
36
],
"console:src/components/PopupWrapper/PopupWrapper.tsx": [
47,
56
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same explanation for this file as above

Comment on lines +2 to +4
"desktop:app/src/ui/app-menu/app-menu-bar-button.tsx": [
190
],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This seems to be a false positive stemming from using React components and us not understanding it. This was first reported here - https://community.sonarsource.com/t/s6819-prefer-tag-over-role-is-misleading/107348/7 and we have a jira task to track these - #4647

I'm not sure why it just now starting showing up in the ruling, but I think it's fair.

Comment on lines +5 to +10
"moose:renderer/components/Downloads/Downloads.tsx": [
130
],
"moose:renderer/components/FilesList/FilesList.tsx": [
165
],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a true-positive, that wasn't triggering before
image

Comment on lines +9 to +10
165
],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

True-positive

image

Comment on lines -3 to -4
19,
26,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

same explanation as above

Copy link

Quality Gate failed Quality Gate failed

Failed conditions
75.0% Coverage on New Code (required ≥ 90%)

See analysis details on SonarQube

@zglicz zglicz merged commit a02409f into master Sep 19, 2024
14 of 16 checks passed
@zglicz zglicz deleted the js-261 branch September 19, 2024 12:11
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.

2 participants