-
Notifications
You must be signed in to change notification settings - Fork 79
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
[JAVA] [EC10] Image vectors #154
base: main
Are you sure you want to change the base?
Conversation
Kudos, SonarCloud Quality Gate passed! |
Hi @PheonBest, please could you tell us where do you find this list of superfluous attributes ? I'm curious :p thx |
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.
please add also updates with this rule implementation :
CHANGELOG.md
: add issue linkRULES.md
: change icon
another modification to do : add a tests in ecoCode-java-test-project ==> please check others tests and make the same. as you can see test files are quite identical to unit test resources files
please make corrections on code smell problems (sonarcloud report)
@@ -30,7 +30,7 @@ services: | |||
- "extensions:/opt/sonarqube/extensions" | |||
- "logs:/opt/sonarqube/logs" | |||
- "data:/opt/sonarqube/data" | |||
|
|||
restart: unless-stopped |
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.
what is the use case for this need ?
for me, if container crashes, the developer must check why on logs et make a correction or an issue for correction.
if we add this option, we could miss the real pb
@@ -0,0 +1,161 @@ | |||
package fr.greencodeinitiative.java.checks; |
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.
indeed, this new rule implementation isn't included in ecoCode plugin because there is no declaration in RulesList
class : this is mandatory to include it.
import java.util.stream.Stream; | ||
|
||
@Rule( | ||
key = "EC53", |
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.
please, check other rule implementation to make the same :
- create a constant with id value (to make it easier to use it in other classes)
- tags : no
bug
, butecocode
,eco-design
andperformance
(maybe other tag, please check available native tags on SonarQube
public class OptimizeImageVectorsSize extends IssuableSubscriptionVisitor { | ||
protected final String SVG_BEGIN = "<svg"; | ||
protected final String[] SVG_END = new String[]{"</svg", "</ svg>"}; | ||
protected final List<String> SUPERFLUOUS_ATTRIBUTES = Arrays.asList("xmlns:dc", "xmlns:cc", "xmlns:rdv", "xmlns:svg", "xmlns", "xmlns:sodipodi", "xmlns:inkscape", "inkscape:version", "inkscape:label", "inkscape:groupmode", "sodipodi:docname"); |
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.
please could you tell us where do you find this list of superfluous attributes ? I'm curious :p
If we are ok with you, we could report it to other language rule implementation
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.
Hello @dedece35
I extracted the superfluous attributes from SVGGO demo page, which provides an original & compressed svg file that I compared:
compressed: blob:https://jakearchibald.github.io/09bc7d0d-2148-470d-9a25-86355304127f
original: blob:https://jakearchibald.github.io/b11f1eec-fd24-4bf2-95f2-127555093714
It's not a perfect list !
You can also define custom attributes in svg, e.g: version='1.2'
These attributes are superfluous if they're not used in the rest of the svg.
} | ||
|
||
@Override | ||
public void visitNode(Tree tree) { |
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.
please, explode this method in several smaller private functional methods. this is for readability and ease to maintain it.
return; // svg is invalid or is broken into multiple strings | ||
} | ||
|
||
// Parse svg as xml and explore its tree |
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.
why do you use XML parsing and not simplier way, for example string pattern search on all java code ?
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.
I looked into regex matching at first, but I realized it may not suit some rules I defined.
The name of some superfluous attributes can be used as content/text instead of tags attributes, e.g: xmlns.
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.
Maybe we should consider using XPath? SVGs files being XML, this would allow us to perform the analysis independently of what language/framework is used. This is mentionned in the SonarQube documentation
import java.util.stream.Stream; | ||
|
||
@Rule( | ||
key = "EC53", |
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.
wrong rule id ==> EC10
This PR has been automatically marked as stale because it has no activity for 30 days. |
Hi @PheonBest, and please make a correction of your Jav files to inlucde mandatory licence header like other Java files (that's why the verify phase of test process is bad) |
This PR has been automatically marked as stale because it has no activity for 30 days. |
This PR has been automatically marked as stale because it has no activity for 60 days. |
Idea: Use less heavy SVG images using less bandwidth (for Spring Boot applications, etc .)
Related issue: #111
This rule anlyzes unoptimized svg strings by using XPath.
The rule detects the following unoptimizations: