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

docs(certificate-management): created new certificate management kit #684

Merged
merged 4 commits into from
Mar 6, 2024

Conversation

SujitMBRDI
Copy link
Contributor

Description

With this pull request, created new KIT for business partner certificate management application.
Newly added KIT contains changelog, adaption view, operation view, development view and documentation for architecture of application.

Pre-review checks

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

@maximilianong
Copy link
Contributor

@SujitMBRDI please check the linting error.

@SujitMBRDI
Copy link
Contributor Author

@SujitMBRDI please check the linting error.

Hi @maximilianong, Linting is not failing because of changes in this pull request.
I already checked the warning and error, this is happening because of other KIT's linting issue.
Multiple pull requests are affected due to this.

Copy link
Contributor

@maximilianong maximilianong left a comment

Choose a reason for hiding this comment

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

Content reviewed for release 24.03.
Please clarify the inline comments.
And have a look into the release-issue: eclipse-tractusx/sig-release#537

Changes need to be adapted til the 4th of March.

@maximilianong
Copy link
Contributor

@SebastianBezold we reviewed the content. If you have time - can you review it technically because it's a new KIT. Thanks.

maximilianong
maximilianong previously approved these changes Feb 22, 2024
Copy link
Contributor

@maximilianong maximilianong left a comment

Choose a reason for hiding this comment

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

Thanks @SujitMBRDI for adapting the changes.

@maximilianong
Copy link
Contributor

@SujitMBRDI you please resolve the conflict - we wanted to merge today to prepare the release.

@SujitMBRDI
Copy link
Contributor Author

@SujitMBRDI you please resolve the conflict - we wanted to merge today to prepare the release.

Hi @maximilianong, i have resolved the conflict!
This happened as ESS kit merged before.

@SujitMBRDI SujitMBRDI requested a review from maximilianong March 6, 2024 08:08
Copy link
Contributor

@SebastianBezold SebastianBezold left a comment

Choose a reason for hiding this comment

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

You should simplify static asset references.
I'm not sure, why they are always referenced via @site.
See the Docusaurus docs on static assets and how to reference them in markdown.

Additionally: Please rethink your directory and filenames. Using space characters in websites just makes it harder to reference stuff, since it always has to be URL encoded.. You are also mixing kebap-case and snake case. I would suggest to stick with kebap case

Copy link
Contributor

Choose a reason for hiding this comment

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

Is it necessary, that this file is .mdx? I don't see anything specific here.
The image references should not use @site, but just absolute paths.
See the Docusaurus documentation for static assets.

Example:

instead of @site/static/img/Certificate_Kit_Icon.png , just use /img/Certificate_Kit_Icon.png

@SujitMBRDI
Copy link
Contributor Author

You should simplify static asset references. I'm not sure, why they are always referenced via @site. See the Docusaurus docs on static assets and how to reference them in markdown.

Additionally: Please rethink your directory and filenames. Using space characters in websites just makes it harder to reference stuff, since it always has to be URL encoded.. You are also mixing kebap-case and snake case. I would suggest to stick with kebap case

Hi @SebastianBezold,
I have adapted changes for image reference and removed .mdx files.
Also, made the changes for file names kept Kebab case.
Kindly have a look. Initially, i thought of same points but when i observed other KITs it was developed in that fashion so kept it.

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.

5 participants