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

Change industry core logos #1074

Conversation

Grand-Thibault
Copy link
Contributor

@Grand-Thibault Grand-Thibault commented Nov 7, 2024

Description

  • Industry Core is an own domain and should appear as such
  • Own color scheme for IC Kits

Pre-review checks

Please ensure to do as many of the following checks as possible, before asking for committer review:

Copy link
Member

@jSchuetz88 jSchuetz88 left a comment

Choose a reason for hiding this comment

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

Hi @Grand-Thibault,

  • I think you missed to update the url to the new images in ./kitsGallery.js.
    Somewhere in line 20 to 38.

  • You need to add licences to each .svg file (see TRG 7.02): e.g. data-chain-kit-gallery.drawio.svg.licence with the respective licence

Additional remark:
You changed the images only for the upcoming release (R24.12). Is that on purpose? usually the gallery icons were consistent across the releases.

Copy link
Member

@jSchuetz88 jSchuetz88 left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@tom-rm-meyer-ISST
Copy link
Contributor

First of all: thanks a lot! Same png / svg issue as in #1084

Also a question to @stephanbcbauer as he may have an overview about refactorings here: Which is the targeted structure for image assets? All into the respective KIT incl. kit icons (as started here) or just putting them into the current folders.

just want to be aware of "is there a strategy"? If there is, we need to somewhat explain it in the office hours / dev mailing list.

@stephanbcbauer
Copy link
Member

First of all: thanks a lot! Same png / svg issue as in #1084

Also a question to @stephanbcbauer as he may have an overview about refactorings here: Which is the targeted structure for image assets? All into the respective KIT incl. kit icons (as started here) or just putting them into the current folders.

just want to be aware of "is there a strategy"? If there is, we need to somewhat explain it in the office hours / dev mailing list.

@tom-rm-meyer-ISST THX for your comment. Is it really the case SVG is not rendered any more? Since I thought this was the idea to use them. Exporting the PNG would mean we have 4 files for one image?

  1. SVG as source to be able to adapt it
  2. PNG Exported picture
  3. License file for both?

Related to the structure, depends on if the image/icon should be versioned or not. For example, diagrams should be included in the KITs folder since the relates to specific version. If there is a static images, which are not versioned, we should keep it in the static folder. But, we should probably structure the static/img folder a little bit better?

@Grand-Thibault
Copy link
Contributor Author

[...] But, we should probably structure the static/img folder a little bit better?

Defininetly! As the other folders are structured mostly according to the KITs, I would do the same here.

@tom-rm-meyer-ISST
Copy link
Contributor

@stephanbcbauer exactly, same here.
@Grand-Thibault overall it's a great idea, but we need to either directly do it for all OR make everyone aware of that change, so they'll do during next PRs. I only see the issue with the latter, that it's not going to happen, if we don't announce it.

If you both prefer, the document structure n this PR, then go for it and ignore my comments in the other PR! :)

Sidenote: Please remember to remove the old images, too :)

Copy link
Contributor

@tom-rm-meyer-ISST tom-rm-meyer-ISST left a comment

Choose a reason for hiding this comment

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

Please check license names

Copy link
Contributor

@tom-rm-meyer-ISST tom-rm-meyer-ISST left a comment

Choose a reason for hiding this comment

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

fixed an issue that arised from removing the scdn png

@tom-rm-meyer-ISST tom-rm-meyer-ISST merged commit 2eb3ec9 into eclipse-tractusx:main Nov 20, 2024
5 checks passed
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.

4 participants