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

Describe how to use build artifact from a docu repo PR to create the webpage #456

Merged
merged 32 commits into from
Apr 26, 2024

Conversation

pgfeller
Copy link
Contributor

@pgfeller pgfeller commented Mar 6, 2024

Fixes #455

This PR adds additional functionality to build the website using changes from a PR from the documentation repository.

Contributors:

Special Thanks to:


In special situations; e.g. if you make changes in the documentation repository that might impact the website build, it is possible to build the website from a PR. This makes it possible to verify that your proposed changes do not have negative side effects:

ARGUMENTS="--pull-request 2272" npm run build-local

Available ARGUMENTS:

  • --verbose
  • --no-clone
  • --pull-request #

@pgfeller pgfeller marked this pull request as ready for review March 10, 2024 22:55
@pgfeller
Copy link
Contributor Author

@florian-h05 and @stefan-hoehn - please have a look at the proposed changes. This PR adds some arguments to create and serve the documentation page locally based on a PR. It needs some fine tuning - but I would appreciate your feedback if this adds value add all - or if the "feature" is not necessary.

ARGUMENTS="--pull-request 2272" npm run build-local

> [email protected] prebuild-local
> npm run prepare-docs


> [email protected] prepare-docs
> ruby prepare-docs.rb $ARGUMENTS

➡️ Cloning repository 📦 dilyanpalauzov:oh_addons_install ...
  ↪️ PR #2272: Document addon installation in Karaf from Marketplace
➡️ Migrating the introduction article
➡️ Migrating common images
➡️ Migrating logos
➡️ Migrating the Concepts section
➡️ Migrating the Installation section
➡️ Migrating the Tutorial section
➡️ Migrating the Configuration section
➡️ Migrating the Main UI section
➡️ Migrating the Migration Tutorial section
➡️ Migrating the Blockly Tutorial section
➡️ Migrating the UI section
process_file: IGNORING (NON-EXISTING): .vuepress/openhab-docs/_addons_uis/habpanel/doc/habpanel.md
process_file: IGNORING (NON-EXISTING): .vuepress/openhab-docs/_addons_uis/habot/readme.md
➡️ Migrating the Apps section
➡️ Migrating the Administration section
➡️ Migrating the Developer section
➡️ Migrating add-ons: Automation
➡️ Migrating add-ons: Persistence
➡️ Migrating add-ons: Transformations
➡️ Migrating add-ons: Voice
➡️ Migrating add-ons: IO
➡️ Migrating add-ons: UI
⚠️  Ecosystem documentation for [Alexy, Mycroft, Google] skipped - depends on Git-Workflow ...
➡️ Migrating add-ons: Bindings
➡️ Creating ZWave thing viewer
➡️ Writing add-ons arrays to files for sidebar navigation
⚠️  Iconsets depend on Git-Workflow - will be skipped ...
➡️ Downloading and extracting latest Javadoc from Jenkins
2024-03-10 23:58:12 URL:https://ci.openhab.org/job/openHAB-JavaDoc/lastSuccessfulBuild/artifact/target/javadoc-latest.tgz [4357840/4357840] -> "javadoc-latest.tgz.8" [1]
⚠️  Thingtypes depend on Git-Workflow - will be skipped ...

> [email protected] build-local
> npm run dev --silent

image

... let me know if I shall continue - or if we abandon this, because we do not need it. In that case we might consider to salvage some parts (e.g. improved processing of arguments in the ruby script.

with kind regards

@stefan-hoehn
Copy link
Collaborator

I think that the idea definitely makes sense and I like it a lot. The more and easier we can try out changes locally the better it is. 😍

In general it looks good to me. I would recommend having a joint session for trying it out together.

@florian-h05 I would appreciate if you have a closer look at the code as well.

package.json Show resolved Hide resolved
prepare-docs.rb Outdated Show resolved Hide resolved
prepare-docs.rb Outdated Show resolved Hide resolved
prepare-docs.rb Show resolved Hide resolved
prepare-docs.rb Show resolved Hide resolved
.vuepress/.gitignore Show resolved Hide resolved
prepare-docs.rb Show resolved Hide resolved
prepare-docs.rb Show resolved Hide resolved
prepare-docs.rb Outdated Show resolved Hide resolved
prepare-docs.rb Outdated Show resolved Hide resolved
Copy link
Contributor

@florian-h05 florian-h05 left a comment

Choose a reason for hiding this comment

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

In general I really like this PR and I think it will prove helpful 👍

A few first comments, I have had a look at all files except the Ruby script.
I will review the Ruby script as well once you have addressed Stefan‘s comments.

.vuepress/.gitignore Show resolved Hide resolved
.vuepress/config.js Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
package.json Show resolved Hide resolved
@pgfeller pgfeller changed the title Describe how to use build artifact from a docu repo PR to create the webpage [WIP] Describe how to use build artifact from a docu repo PR to create the webpage Mar 27, 2024
@pgfeller
Copy link
Contributor Author

pgfeller commented Mar 27, 2024

I've some pending tasks before a merge can be done:

  • - update to origin/HEAD and resolve conflicts
  • - check, if the feature(s) still works after the changes by the code review & merge/update
    @stefan-hoehn & @florian-h05 - use ARGUMENTS="--pull-request 2277" npm run build-local if you want to try it in action on your system(s) - it is always good to know if it also works on another machine ....

➡️ some of the review comments I did not close yet ... please check if you are happy with the change(s) of the open comments (then I close them) - or let me know what needs to be improved.

Copy link
Contributor

@florian-h05 florian-h05 left a comment

Choose a reason for hiding this comment

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

Overall looks good to me, I had a few findings during review which I packed into a PR.
Please have a look at my PR pgfeller#1 and merge it.

pgfeller added a commit to pgfeller/website that referenced this pull request Mar 29, 2024
@pgfeller pgfeller changed the title [WIP] Describe how to use build artifact from a docu repo PR to create the webpage Describe how to use build artifact from a docu repo PR to create the webpage Mar 29, 2024
Signed-off-by: Patrik Gfeller <[email protected]>
Signed-off-by: Patrik Gfeller <[email protected]>
Signed-off-by: Patrik Gfeller <[email protected]>
Signed-off-by: Patrik Gfeller <[email protected]>
Signed-off-by: Patrik Gfeller <[email protected]>
Signed-off-by: Patrik Gfeller <[email protected]>
Signed-off-by: Patrik Gfeller <[email protected]>
Signed-off-by: Patrik Gfeller <[email protected]>
Signed-off-by: Patrik Gfeller <[email protected]>
Signed-off-by: Patrik Gfeller <[email protected]>
Signed-off-by: Patrik Gfeller <[email protected]>
Signed-off-by: Patrik Gfeller <[email protected]>
Signed-off-by: Patrik Gfeller <[email protected]>
@pgfeller
Copy link
Contributor Author

@florian-h05 - many thanks for your clean ups 👍

➡️ @stefan-hoehn & @florian-h05 : ... from my side this PR should be ready to merge. I'll have a look into on #2216 next to see if we can use the official footer slot of the template for the edit link to properly use footnotes. I'll also add what markdown extensions we support to the contributor .md

Have a nice (long) weekend & ping me if I missed something in this PR.

@stefan-hoehn - as you may have noticed: I like icons - my favorite console font(s): Nerd Fonts

Copy link
Contributor

@florian-h05 florian-h05 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!!

Just one minor question, and could you please also update https:/openhab/website/blob/fbf16d57b01a38690cb53efd57ddf84b4b45e0dc/add-blog-meta.rb with the arrow icons to make it consistent?

@stefan-hoehn After this PR has been merged I would update the Netlify build setting to to set the verbose argument.

package-lock.json Outdated Show resolved Hide resolved
@pgfeller
Copy link
Contributor Author

https:/openhab/website/blob/fbf16d57b01a38690cb53efd57ddf84b4b45e0dc/add-blog-meta.rb with the arrow icons to make it consistent?

Thank you for the review 👍.

Copy link
Contributor

@florian-h05 florian-h05 left a comment

Choose a reason for hiding this comment

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

LGTM now, thanks for the contribution 👍
Just one minor formatting comment left.

@stefan-hoehn Feel free to merge once you have had a look.

add-blog-meta.rb Outdated Show resolved Hide resolved
Co-authored-by: Florian Hotze <[email protected]>
Signed-off-by: Patrik Gfeller <[email protected]>
Copy link
Collaborator

@stefan-hoehn stefan-hoehn left a comment

Choose a reason for hiding this comment

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

After talking with @florian-h05 that this has been tested, I approve it now. Thanks to @pgfeller and @florian-h05 for putting so much work and thoughts into it.

@stefan-hoehn stefan-hoehn merged commit e0ff40e into openhab:main Apr 26, 2024
5 checks passed
florian-h05 added a commit to florian-h05/website that referenced this pull request May 31, 2024
openhab#456 made the prepare-docs step configurable through arguments passed in with the ARGUMENTS env var.
However did the npm build script not respect them, this fixes this.

Signed-off-by: Florian Hotze <[email protected]>
stefan-hoehn pushed a commit that referenced this pull request May 31, 2024
#456 made the prepare-docs step configurable through arguments passed in with the ARGUMENTS env var.
However did the npm build script not respect them, this fixes this.

Signed-off-by: Florian Hotze <[email protected]>
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.

Describe how to use build artifact from a docu repo PR to create the webpage
3 participants