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

Adding Aquasec icon. #1323

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

Conversation

Mulgish
Copy link
Contributor

@Mulgish Mulgish commented Jul 30, 2022

Adding Aquasec icon following jenkinsci/analysis-model#820.

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

@@ -254,3 +254,8 @@ The Stylelint logo is licensed under CC BY 4.0.
https://icon-icons.com/icon/Stylelint/131966

------------------------------------------------------------------------------

The Aquasec logo is licensed under the GNU GENERAL PUBLIC LICENSE.
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the GNU GPL license text then have to be distributed with the plugin?

Is the SVG form the source, or is there a different preferred form for editing?

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, GPL is not compatible with MIT as far as I understand. On https://www.aquasec.com/brand/ I do not find the GPL reference (but there are only other tools listed). Is there a way to contact them if it's ok to reuse the icon here?

Copy link
Contributor

Choose a reason for hiding this comment

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

If you do contact them, I suggest you send them a screen shot of how the logo would look in Jenkins when accompanied by other logos. Mainly to show how much space is available and whether it is feasible to include the "aqua" logotype that they have on the branding page.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will try to contact them at some point.
In the meantime Stylelint icon might need a review as well as it is using https://icon-icons.com/icon/Stylelint/131966.

I am curious if it would be better if we generated icons ourselves based on tool abbreviations?
That way it makes it easier to distinguish them on the UI rather than seeing the default icon.

Copy link
Contributor

Choose a reason for hiding this comment

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

stylelint/stylelint#340 imported the logo "under CC license" but I don't know which of the CC licenses that means. https://github.com/stylelint/stylelint/blob/14.9.1/identity/credits.md links to https://thenounproject.com/icon/tuxedo-2104/ but I don't see license terms on that page. https://thenounproject.com/legal/ section 3(A) says different icons are available under different licenses. Perhaps the site requires users to sign in before it reveals the license for each icon.

Copy link
Contributor Author

@Mulgish Mulgish Jul 31, 2022

Choose a reason for hiding this comment

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

Looks like it has CC BY license.

image
image

@codecov
Copy link

codecov bot commented Jul 30, 2022

Codecov Report

Merging #1323 (6cbd22f) into master (ae60678) will increase coverage by 0.09%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master    #1323      +/-   ##
============================================
+ Coverage     79.59%   79.68%   +0.09%     
- Complexity     1444     1446       +2     
============================================
  Files           249      249              
  Lines          5532     5533       +1     
  Branches        426      426              
============================================
+ Hits           4403     4409       +6     
+ Misses          978      974       -4     
+ Partials        151      150       -1     
Impacted Files Coverage Δ
...jenkins/plugins/analysis/warnings/AquaScanner.java 85.71% <100.00%> (+2.38%) ⬆️
...ns/plugins/analysis/core/model/AnalysisResult.java 89.57% <0.00%> (+3.06%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us.

@@ -254,3 +254,8 @@ The Stylelint logo is licensed under CC BY 4.0.
https://icon-icons.com/icon/Stylelint/131966

------------------------------------------------------------------------------

The Aquasec logo is licensed under the GNU GENERAL PUBLIC LICENSE.
https://icon-icons.com/icon/aquasec-logo/170524
Copy link
Contributor

Choose a reason for hiding this comment

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

That site also lists the Apple logo as being GPL licensed. Doesn't seem trustworthy to me. https://icon-icons.com/icon/apple-logo/168588

Copy link
Contributor

Choose a reason for hiding this comment

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

icon-icons.com seems to have copied the aquasec logo from https://github.com/VectorLogoZone/vectorlogozone/blob/84a142101777f3f9131d9e2f326e8e15275fa0b9/www/logos/aquasec/aquasec-icon.svg, first committed on 2018-08-22. LICENSE.TXT in that repository was changed from GNU GPL v3-or-later to GNU AGPL v3-or-later on 2018-04-07, so icon-icons.com seems to be operating with out-of-date information. Also, the README.md says "The logos themselves remain property of their original owners." so the LICENSE.TXT doesn't even apply to the logos.

@uhafner
Copy link
Member

uhafner commented Jul 31, 2022

I am curious if it would be better if we generated icons ourselves based on tool abbreviations?
That way it makes it easier to distinguish them on the UI rather than seeing the default icon.

Yes, this is an interesting idea. I'm planning to add a toggle to switch all icons to black and white ones that match better with Jenkins new design (and that are theme aware). Since most tools do not offer black and white iccons I need to replace them with something new (or with the default triangle warning icon).

@uhafner uhafner marked this pull request as draft August 9, 2022 21:30
@uhafner
Copy link
Member

uhafner commented Sep 20, 2022

I started a PR that introduces some symbols now, see #1356.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants