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

front: add some data sources and licenses #8904

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

clarani
Copy link
Contributor

@clarani clarani commented Sep 17, 2024

closes #7621

  • add some data sources and licenses
  • refacto the licenseAttribution component to separe the data sources and the licenses

image

@clarani clarani requested a review from a team as a code owner September 17, 2024 16:32
@codecov-commenter
Copy link

codecov-commenter commented Sep 17, 2024

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 0% with 72 lines in your changes missing coverage. Please review.

Project coverage is 37.13%. Comparing base (2d6138c) to head (b5ac4f7).
Report is 9 commits behind head on dev.

Files with missing lines Patch % Lines
...common/ReleaseInformations/LicenseAttributions.tsx 0.00% 71 Missing ⚠️
...common/ReleaseInformations/ReleaseInformations.tsx 0.00% 1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff              @@
##                dev    #8904      +/-   ##
============================================
+ Coverage     36.98%   37.13%   +0.14%     
  Complexity     2211     2211              
============================================
  Files          1259     1260       +1     
  Lines        114565   115096     +531     
  Branches       3192     3230      +38     
============================================
+ Hits          42374    42737     +363     
- Misses        70290    70426     +136     
- Partials       1901     1933      +32     
Flag Coverage Δ
core 74.67% <ø> (-0.13%) ⬇️
editoast 72.56% <ø> (+0.12%) ⬆️
front 15.19% <0.00%> (+0.22%) ⬆️
gateway 2.20% <ø> (ø)
osrdyne 2.60% <ø> (ø)
railjson_generator 87.49% <ø> (ø)
tests 86.46% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@Yohh Yohh left a comment

Choose a reason for hiding this comment

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

nice!

(this old component obviously did need a refacto 😉)

a few comments that seems important

@Yohh
Copy link
Contributor

Yohh commented Sep 17, 2024

I wonder if we could/should add a copy of the licenses in the project

- add some data sources and licenses
- refacto the licenseAttribution component to separe the data sources and the licenses
Copy link
Contributor

@Yohh Yohh left a comment

Choose a reason for hiding this comment

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

LGTM

Note

we should consider my last question, even if it can be totally unnecessary

Copy link
Contributor

@SharglutDev SharglutDev left a comment

Choose a reason for hiding this comment

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

Thank you for the PR, left some comments :)

const LicenseCard = ({
attribution: { name, version, copyright, publisher, url, license },
}: LicenseCardProps) => (
<a key={name} href={url} rel="noreferrer">
Copy link
Contributor

Choose a reason for hiding this comment

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

There is a warning in console

Capture d’écran 2024-09-19 à 09 18 44

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

…ces and licenses - refacto the licenseAttribution component to separe the data sources and the licenses
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.

Add data sources and licenses
4 participants