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

Relocate usage documentation with local images #440

Merged
merged 3 commits into from
Dec 13, 2021

Conversation

Confectrician
Copy link
Contributor

@Confectrician Confectrician commented Dec 12, 2021

This will:

  • Move the Usage file to it's own subfolder docs like it is done in other openHAB repositories (googel assistant / openhabian / ...)
  • Prepare an images folder within the docs like it is done in other repositories
  • Intorduce a prefix oh_alexa for the doc files

A coordinated merge would be neat, because this will break the current Jenkins CI Job for generating the docs website.
Therefore i have prepared openhab/openhab-docs#1698
Would be nice if we can merge both PRs at the same time to keep the build job running continous.

Goal

  • Generalize how docs and it's images are managed in the different openHAB repos
  • Provide a way to get doc changes into our stable homepage version for apps with an indipendent release cycle

Follow Up (in another PR)

  • Introduce a GitHub Action to trigger a docs update, when the alexa skill has a new release.

For this i need to have the structure and file prefixes in place.

Signed-off-by: Jerome Luckenbach <github@luckenba.ch>
@jsetton
Copy link
Collaborator

jsetton commented Dec 12, 2021

If we want to keep it consistant, do we need to change the file name as well? As a matter a fact, do we actually need to prefix the files with oh_alexa since being in the openhab-alexa repository is self-explanatory?

I proposed to have docs/USAGE.md and the logo image as docs/images/skill-logo.png.

@jsetton jsetton self-requested a review December 12, 2021 15:53
@jsetton
Copy link
Collaborator

jsetton commented Dec 12, 2021

Introduce a GitHub Action to trigger a docs update, when the alexa skill has a new release.

I missed that part. Does this mean the plan is to move away from the Jenkins CI job for gathering external documentation on a scheduled basis?

@Confectrician
Copy link
Contributor Author

Yes, when you are in this repo anyway the prefix seems to be redundant.
But i need prefixes, for the actions placed in the openhab docs repository for my follow up PR.

You can check the openhabian repo for reference.
We added the actions over there as a proof of concept with:
openhab/openhab-docs#1690
openhab/openhabian#1633

@Confectrician
Copy link
Contributor Author

Confectrician commented Dec 12, 2021

I missed that part. Does this mean the plan is to move away from the Jenkins CI job for gathering external documentation on a scheduled basis?

No. The jenkins job will be continued.
With this changes we will have the ability to add changes to the stable docs for apps like alexa or the openhabian image parallel to the jenkins job.

The current problem is, that jenkins only merges contents into the next.openhab.org homepage.
So we have a lack of up to date docs for apps and repos with an indipendet release cycle.

Edit:
And another advantage (from my view) is, that we can then trigger alexa doc updates, when aleaxa has a new release automatically.
Currently we are dependent from the openHAB distro builds completely.

@jsetton
Copy link
Collaborator

jsetton commented Dec 12, 2021

No. The jenkins job will be continued.

My consistency comment was based on that job and the naming convention used by other external documentation integrations (e.g. Google Assistant, Android, ...). Are you planning to have the relevant repositories updated as well?

With this changes we will have the ability to add changes to the stable docs for apps like alexa or the openhabian image parallel to the jenkins job.

I still don't understand the need of a prefix. From my understanding, the GitHub Action event-type job property looks to indicate the source of where to get the documentation unless I am missing something.

The current problem is, that jenkins only merges contents into the next.openhab.org homepage.
So we have a lack of up to date docs for apps and repos with an indipendet release cycle.

I have mixed feeling about it since I am glad that the current documentation isn't live. Even though a new release is cut on the skill side, it takes quite a bit of time to go live since it has to go through a certification process on the Amazon side.

For this one, we would probably tie the documentation updating job on a schedule opposed to right when the release is published.

@jsetton
Copy link
Collaborator

jsetton commented Dec 12, 2021

I still don't understand the need of a prefix. From my understanding, the GitHub Action event-type job property looks to indicate the source of where to get the documentation unless I am missing something.

So I see now where the prefix is used.

It seems to be overkill to enforce this since the repository is already known at that point and any markdown and image files under the docs/ folder should be downloaded. Why not append that prefix on downloaded files instead since it seems that these all go into the same folder? And that prefix could just be the repository name removing the need for doc_base_name and image_base_name parameters.

@jsetton
Copy link
Collaborator

jsetton commented Dec 12, 2021

Never mind, I forgot about document links being affected by what I am proposing. So I understand the need for the prefix now.

The last point I am confused about is that the GitHub Action job updates the installation/ folder while the Jenkins CI updates the _addons_ios/alexa-skill/ folder.

@Confectrician
Copy link
Contributor Author

Are you planning to have the relevant repositories updated as well?

Yes i want to update all of them step by step.
Since jenkins will still be available for the snapshot docs, i may need to adapt the docs repository for each skill/app/repo.
So i think it easier to concentrate on one repo per change.

The last point I am confused about is that the GitHub Action job updates the installation/ folder while the Jenkins CI updates the _addons_ios/alexa-skill/ folder.

I will check if i missed something in the base action again or made a mistake, but in theory this should be fully configurable in the specific action for this repository later, which is not available yet.

Here are the parts of my base action that are relevant:

- name: Copy newest doc contents
       run: |
         echo "Copy current doc contents"
         cp -v ./external_contents/${{inputs.base_source_repository}}/docs/${{inputs.doc_base_name}}*.md ${{inputs.base_folder}}
# Copy images only if flag is set to true
- name: Copy newest images
       if: ${{ inputs.has_images == true}}
       run: |
         echo "Copy current images"
         cp -v ./external_contents/${{inputs.base_source_repository}}/docs/images/${{inputs.image_base_name}}* ${{inputs.base_folder}}/images

So in the next step i would create a new Fetch alexa docs action (parallel to the existing openhabian one) which then uses the base action with a different config for ${{inputs.base_folder}} and the base action should then copy the file to the right place.

I have mixed feeling about it since I am glad that the current documentation isn't live. Even though a new release is cut on the skill side, it takes quite a bit of time to go live since it has to go through a certification process on the Amazon side.

For this one, we would probably tie the documentation updating job on a schedule opposed to right when the release is published.

Glad that you bring this up.
This is one reason, why i have choosen the current setup with several actions.

We will have an action in the alexa repository (like openhab/openhabian#1633) and if you have strong arguments against release as a trigger, this is perfectly fine for me and we will fire the action on different parameters than the openhabian one. :)

The Repo owners or repo main contributors have way more knowledge and experience of their apps and when the docs should get into the stable homepage.
But we can discuss that in detail, when i propose the PR here, with the needed GitHub Action and find the triggers/parameters that fit the best for the alexa repository. (And of course we can change it at any point if it doesn't fit the needs anymore.)

@jsetton
Copy link
Collaborator

jsetton commented Dec 12, 2021

Thanks for the detailed explanation.

So in the next step i would create a new Fetch alexa docs action (parallel to the existing openhabian one) which then uses the base action with a different config for ${{inputs.base_folder}} and the base action should then copy the file to the right place.

If I understood correctly, we would set ${{inputs.base_folder}} as _addons_ios/alexa-skill/ which is where the current documentation is located under the final-stable branch. If this is the case, the prefix wouldn't be necessary since it has its own folder. Am I missing something?

@jsetton
Copy link
Collaborator

jsetton commented Dec 12, 2021

If the doc_base_name and image_base_name parameters are made optional, the job for this repository would look as:

call-reusable-fetch-workflow:
    uses: openhab/openhab-docs/.github/workflows/fetch_external_docs_reusable.yml@main
    with:
      base_source_repository: openhab-alexa
      base_folder: _addons_ios/alexa-skill
      has_images: true

As a side note, I would make has_images optional as well defaulting to false.

@Confectrician
Copy link
Contributor Author

If this is the case, the prefix wouldn't be necessary since it has its own folder. Am I missing something?

Nope. I missed the deviating folder structure of the installation folder where openhabian is placed.

If every repo would use its own prefix this would be easier for me and a bit more future proof in my view.
Is this really such a big problem?

If possible (i have to check the maven conf for jenkins for that) i would like to get rid of those many extra folders on long term.
But for now we have to implement the action in a way that jenkins can run parallel.
So i have to add exclusions/conditions in the base action anyway.

@jsetton
Copy link
Collaborator

jsetton commented Dec 12, 2021

If every repo would use its own prefix this would be easier for me and a bit more future proof in my view.
Is this really such a big problem?

First, I am not a fan of prefixing all files in the source repository. It's fine if you have one or two files but if you have a bunch of images, it would start to over complicate things.
Secondly, if you intend to migrate the other external documentation, you wouldn't have to change anything besides adding the GitHub Action workflow.
Thirdly, from my understanding, the prefix is only used to move all markdown and image files under a single endpoint to prevent conflicts between different projects. Is that necessary? Why can't separate folders be kept for each project without the need to remember to add a prefix whenever making local document changes to a given project?

@jsetton
Copy link
Collaborator

jsetton commented Dec 13, 2021

@Confectrician any thoughts on what I highlighted above? I would love to add this change in the final 3.2 release.

I am basically suggesting to use the file names listed above.

You would need to make the following changes to fetch_external_docs_reusable.yml:

@@ -30,15 +30,16 @@
         type: string
       doc_base_name:
         description: 'The doc file name prefix for lookup options.'
-        required: true
+        required: false
         type: string
       has_images:
         description: 'Flag for enabling image copy commands.'
-        required: true
+        default: false
+        required: false
         type: boolean
       image_base_name:
         description: 'The image name prefix for lookup options'
-        required: true
+        required: false
         type: string

 jobs:

And add fetch_docs_openhab-alexa.yml as:

name: Fetch OpenHAB Skill For Amazon Alexa Docs

on:
  # Repository dispatch event, to be triggerd by an openhab-alexa release
  # or manually from the openhab-alexa repository
  repository_dispatch:
    types: [update-openhab-alexa-docs-event]
  workflow_dispatch:

jobs:
  call-reusable-fetch-workflow:
    uses: openhab/openhab-docs/.github/workflows/fetch_external_docs_reusable.yml@main
    with:
      base_source_repository: openhab-alexa
      base_folder: _addons_ios/alexa-skill
      has_images: true

If you decide to update the external documentation structure in the future, base_folder would need to be updated to point to the new subfolder location.

@Confectrician
Copy link
Contributor Author

Why can't separate folders be kept for each project without the need to remember to add a prefix whenever making local document changes to a given project?

It can't be kept, since it is not the truth for all external repositories currently.
And keeping openhabain (for example) in the installation section makes still sense.

The problem here is more, that alexa skill and the other integrations live in a completely different homepage area when the website build has taken place, not in the docs area.
This is a grown structure (and problem).

Anyway and to be honest.
I am not willed to discuss 100 posts about some simple prefixes.
This is a big waste of time which both of us could use for some useful changes...

If it is not acceptable to have some file prefixes for whatever reason i will accept this solution....

Signed-off-by: Jerome Luckenbach <github@luckenba.ch>
@jsetton
Copy link
Collaborator

jsetton commented Dec 13, 2021

And keeping openhabain (for example) in the installation section makes still sense.

I agree. I am not asking to change the way each external repositories is structured. I only proposing to add some flexibility and not mirror the openhabian documentation format to all other repositories.

@Confectrician
Copy link
Contributor Author

I have already adapted the file names according to the suggestion and i have also stopped the complete ci pipeline currently for other reasons.

If there are no disadvantages from your side anymore, this would be a good time to make the changes in both repositories.

@jsetton
Copy link
Collaborator

jsetton commented Dec 13, 2021

I just added a few documentation cleanup that I intended to merge as well and also fixed the logo image link.

If there are no disadvantages from your side anymore, this would be a good time to make the changes in both repositories.

I will merge this PR now. Feel free to update your side.

@jsetton jsetton changed the title Adapt docs to other openHAB repositories Relocate usage documentation with local images Dec 13, 2021
@jsetton jsetton merged commit 79275dd into openhab:main Dec 13, 2021
@Confectrician Confectrician deleted the refactor_docs branch December 13, 2021 20:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants