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

feat: add docsify #1012

Merged
merged 5 commits into from
May 12, 2022
Merged

Conversation

juliapampus
Copy link
Contributor

@juliapampus juliapampus commented Mar 30, 2022

What this PR changes/adds

Add docsify as a framework to deploy the current /docs directory as a static website using GitHub pages. As I already had this work done in an old branch from some months ago, I thought directly creating a PR and showing the impact could help with the discussion.

Why it does that

In #580, it was already discussed whether a framework should be used and which one. The most important aspect is: Keep it simple and avoid additional overhead. We only need a rendering of the documentation, no fancy EDC website (this will not be done with GitHub pages).

With the DSC, we used "Just the Docs" (see the deployment here). It worked well and the maintenance was quite easy, but the existing .md files needed to be extended with additional metainformation. Therefore, existing files needed to be modified. Hence, I was looking for a framework that would work without any changes. Whatever framework we choose, all support advanced customization, nevertheless, docsify generates web content by simply using the .md files as they are.

See a sample rendering here (click on "Getting started").

Further notes

As GitHub pages are initially always deployed at the same url, it would not be a problem to change this framework later again - if anyone comes up with a better approach. In addition, this PR only shows the current .md files' content, so empty sections and the general toc structure would have to be filled and modified anyway.

Linked Issue(s)

Closes # <-- insert Issue number if one exists

Checklist

  • added appropriate tests?
  • performed checkstyle check locally?
  • added/updated copyright headers?
  • documented public classes/methods?
  • added/updated relevant documentation?
  • added relevant details to the changelog? (skip with label no-changelog)
  • formatted title correctly? (take a look at the CONTRIBUTING and styleguide for details)

@juliapampus
Copy link
Contributor Author

juliapampus commented Mar 30, 2022

@florianrusch-zf Couldn't add you as a reviewer.

@codecov-commenter
Copy link

Codecov Report

Merging #1012 (2ef6494) into main (705a447) will decrease coverage by 0.15%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##               main    #1012      +/-   ##
============================================
- Coverage     63.09%   62.94%   -0.16%     
  Complexity     2551     2551              
============================================
  Files           633      635       +2     
  Lines         13679    13712      +33     
  Branches        953      957       +4     
============================================
  Hits           8631     8631              
- Misses         4606     4639      +33     
  Partials        442      442              
Impacted Files Coverage Δ
.../micrometer/MicrometerExecutorInstrumentation.java 0.00% <0.00%> (ø)
...nector/metrics/micrometer/MicrometerExtension.java 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 705a447...2ef6494. Read the comment docs.

@florianrusch-zf
Copy link
Contributor

@florianrusch-zf Couldn't add you as a reviewer.

Year, I'm not a contributor so far... :-/
Will have a look at it in the evening.

Copy link
Member

@paullatzelsperger paullatzelsperger left a comment

Choose a reason for hiding this comment

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

looks cool! do you know of any way to automatically add the author to the respective pages?

@juliapampus
Copy link
Contributor Author

looks cool! do you know of any way to automatically add the author to the respective pages?

I will take a look whether docsify comes with a solution. Otherwise, maybe we can add an automatism to add those meta information (last edited, author, modified by, etc.)

Copy link
Member

@ndr-brt ndr-brt left a comment

Choose a reason for hiding this comment

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

@juliapampus can we merge this?

@paullatzelsperger
Copy link
Member

can we merge this? @MoritzKeppler @alexandrudanciu?

Copy link
Contributor

@alexandrudanciu alexandrudanciu left a comment

Choose a reason for hiding this comment

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

Looks great, thank you for the contribution!
The only question I had was if the new EDC logo (the one consisting of connected dots) should be used instead of the incubation logo. But of course, this can be discussed and addressed at a later point.

@juliapampus
Copy link
Contributor Author

juliapampus commented May 11, 2022

@MoritzKeppler @alexandrudanciu @ndr-brt

I will take a look wether everything is up to date later this day. We can merge it afterwards,. In a next step, I will check how we can structure this in a better way.

@juliapampus
Copy link
Contributor Author

Looks great, thank you for the contribution! The only question I had was if the new EDC logo (the one consisting of connected dots) should be used instead of the incubation logo. But of course, this can be discussed and addressed at a later point.

I changed the logo (to edc logo) and chose the green in it as the primary color.

@juliapampus juliapampus merged commit 2f3adc5 into eclipse-edc:main May 12, 2022
@juliapampus juliapampus deleted the feat/add-docsify branch May 12, 2022 07:11
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.

7 participants