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

Global project updates #178

Merged
merged 17 commits into from
Jun 4, 2023
Merged

Global project updates #178

merged 17 commits into from
Jun 4, 2023

Conversation

dlmiles
Copy link
Contributor

@dlmiles dlmiles commented May 7, 2023

  • meta: canonical added
  • meta: robots added (for test builds, google ignore)
  • building 'latest' label as alias to v1.8.0
  • bottom: Version info line
  • github-action builder updates (param stuff, build multiversion)
  • github-action deploy works on local fork
  • top right 'fork me on github'
  • left side version warning box (dismisses for 24h, URL param override)
  • CSS attempt for narrow/mobile compat
  • CI building using a single script for all modes

Needs testing in production.

Then setup of 'on schedule' to autobuild and publish every month, to counter 3 monthly gh-pages retention policy.
Note the default setup will run the linkcheck which may fail at the time of the 'on schedule' maybe skipping linkcheck for the 'on schedule' is more appropriate to recreate the last version as-is.

@dlmiles
Copy link
Contributor Author

dlmiles commented May 8, 2023

Sample of these changes can be found deployed at: https://dlmiles.github.io/SpinalDoc-RTD/

This copy also includes documentation commits that are still WIP can be seen at dlmiles/SpinalDoc-RTD@dlm-202307...dlmiles:SpinalDoc-RTD:dlm-202309 will send PR with those over the coming week.

@dlmiles dlmiles force-pushed the dlm-202307 branch 6 times, most recently from ee5e695 to 2f0a5a6 Compare May 10, 2023 07:01
@Dolu1990
Copy link
Member

Thanks ^^

Let's us know when you are done with the changes :)

@dlmiles dlmiles force-pushed the dlm-202307 branch 3 times, most recently from 98ef358 to ae02768 Compare May 11, 2023 07:33
@dlmiles
Copy link
Contributor Author

dlmiles commented May 11, 2023

This should be ready now, please apply the 'dev' branch PR first and when happy with that look at this one 2nd.

This PR includes an alternative GH action, it sould be safe to auto-pilot, at this time the only way it can affect the live deployment/publish is to run a manual workflow task and tick the box for 'deploy to live'.

DO NOT run the 'deploy to test' mode on the production repository, while the result might look the same:

  • this mode will let you pick any option combination, the live mode won't let you build it with wrong options (must be MULTI)
  • will publish to gh-pages overwriting what is there, even through it is called 'deploy to test',
  • it will have SEO blocking data (for google) telling search engines not to index. This is designed for forks of the project so google does not get confused.

It will run on pull_request (but only a fast 'make html' build, with no-linkcheck, with a ZIP file artifact to download, no publish/deploy occurs). This is designed as am automatic fast test to allow inspection of the output log for any problems.

It will run and deploy (to test) if you push to a branch called 'test', but that branch does not exist in the main repo and is designed for forks to be able to just push to GH and get a result published in 20 mins or so. So don't use that branch name in the main repo.

It can be run manually with 'workflow_dispatch' from github UI, this works in this repo but also in all forks.

If you wanted to use it for a production deployment:
 * Tick the 3 boxes with the word "live" in the description.
 * You can pick any option combination for the 2 linkcheck items, as necessary in the moment**

For example right now, you might need to disable the FAILON_LINKCHECK to it runs and reports but does not block a deployment.

If this is useful and works well, then I would recommend the setup of an "on schedule" task like:

on:
  schedule:
    # * is a special character in YAML
    # setup monthly background build
    - cron: '45 5 20 * *'
    # gh-pages have a lifetime ?  90 days ? so we do this once a month to refresh

This will autopilot a rebuild every month, to defeat the 3 month link for retention on GH Pages.
This will build using latest 'master' and SKIP_LINKCHECK.

As the latest / v1.8.0 is consider the stable branch/tag pushing things to master

I write this out here so it provide a reference if needed in the future.

@andreasWallner
Copy link
Collaborator

Some thoughts:

  • If I got it correctly this would run linkcheck for the cron job and not for a PR? I'm wondering if it would make more sense to turn the linkcheck settings around: do the linkcheck for PRs (only latest singlepage, links should be up to date) and skip it for the cronjob to ensure that we don't stop pushing documentation and it could time out?
  • Did I understand correctly that the workflows are to a) overrule the linkcheck for the normal docs and b) for users to test in their local forks?
  • It would be nice to have some usage notes for the manual targets in the README

@Dolu1990 I would propose to get rid of the current dev branch - there is not much there that is interesting, master is where it's at. Whether we then rename the current master to dev is a different question (so that it matches SpinalHDL), I'd most likely just erase dev...

@dlmiles
Copy link
Contributor Author

dlmiles commented May 13, 2023

top points

  • The thought is the PR it just checking the contribution is well formed, will build, and makes available the errors/warnings from that in the log near the artifact. If someone wanted to inspect how it looks, they would have to download the artifact as it is not published anywhere.
  • The link check setup (this can be changed for any scenario to suite project but, this is the thinking behind current setup):
    • Not run on PR (if you wanted it on PR, maybe disable FAILON because it will fail building for the contribution, when a 3rd party website is down, or link out of date, that has nothing to do with the contribution itself, the contributor might think they have done something wrong).
    • Linkcheck is setup FAILON=enabled for the default options for manual workflow run.
    • When publishing to LIVE you can pick what you want linkcheck to do, SKIP, REPORT, FAIL via the 2 options. The default is as above FAILON (the most conservative/safest)
    • When running on schedule because this is happening to prevent 3 month content timeout, you don't want it failing, even when a link is out of date, you just want it to recreate what was up there before and reset the 3 month retention.
    • When pushing to 'test' in a fork it is setup to SKIP, as I found it a bit excessive to check it everytime, I was pushing a lot, I also found out the failure rate is quite high (all 3rd party links need to be working at the same time as the time it is run). However the forked project can simply edit the setting to suit, or manually run from there to run link check.
  • The link check it more a long term maintenance thing, than a per-contribution thing, it is checking the links in the documentation are not out of date at the remote website. Confirming which ones have redirects on them, possibly ok and intended, possibly because that link is old/dead and moved.
  • I can write up some notes sure, but I did not know if you are welded to the current mechanisms for reasons I am not aware of. But I'd rather get it merged, used, tested, and any understanding resolved, so any revisions can be completed. Before then writing up the process for the production system and some notes to the contributors forking the project (on how it works for them).

dev branch sunset

I agree on the dev branch removal long term goal. I have done the least possible to remove the problems being ~160/many commits behind master maybe causing.
I propose this removal happens in stages to achieve that:

  • Merge 'dev' branch cleanup from other PR (already done)
  • Review/Modify/Merge this PR as necessary
  • Signoff for production use/changes/training/understanding with team members on this (feedback might mean there are further revisions to make with GI actions)
  • Publish LIVE using new process
  • Wait 2 months (for Google SEO to pickup on the new metadata that should be more future proof), so external links pointing to 'dev' by SEO will update.
  • Review content that maybe present in 'dev' and get it moved over to master this can happen in the background while it is still published in the 2 months.
  • Remove the 'dev' from the list of branches/tags used in documentation generation, source/conf.py:235 so a MULTI build does not emit it.
  • Optionally consider to prepare a redirection to latest on any access to /SpinalDoc-RTD/dev/** not sure it is possible, when I researched no server side URL rewrite is possible on gh-pages. So we'd need to generate a skeleton set of static pages to do this.
  • Publish LIVE with that change to remove 'dev' from MULTI
  • Wait 2 months
  • Only then consider any removal through deletion of the dev branch. If at all.

@dlmiles dlmiles temporarily deployed to github-pages May 13, 2023 19:37 — with GitHub Actions Inactive
@dlmiles dlmiles temporarily deployed to github-pages May 13, 2023 23:53 — with GitHub Actions Inactive
@dlmiles dlmiles temporarily deployed to github-pages May 14, 2023 00:05 — with GitHub Actions Inactive
@dlmiles dlmiles force-pushed the dlm-202307 branch 2 times, most recently from 73fc352 to 5f3565b Compare May 14, 2023 01:11
@Dolu1990
Copy link
Member

I would propose to get rid of the current dev branch - there is not much there that is interesting, master is where it's at. Whether we then rename the current master to dev is a different question (so that it matches SpinalHDL), I'd most likely just erase dev...

I'm ok with it, let's work it all on master, and tag it on releases

@andreasWallner
Copy link
Collaborator

As far as I understood the docs will still be published with a merge to master/tag push? Requiring maintainers to trigger the workflow for every merge that they just did does not seem ergonomic - doing the merge should be enough of an indication that we want this to be pushed...

This brings me a bit to the linkcheck: Having that fail may or may not be an issue of some contribution. As you pointed out it might just be some external webpage that is down, or it might be an incorrect link that the contribution just added. I'd rather run it with the risk of some false indications (looking back this is not a frequent occurance) then having it fail when we merged and are then (manually?) pushing the docs.
Since the 90 day rebuild would also not find that this is pretty much the only point in time where we'd see an incorrect link.

@dlmiles
Copy link
Contributor Author

dlmiles commented May 24, 2023

The commit as it is right now should:

  • not affect the process for publishing that exists today. The previous mechanism continues to work. But without the 'latest' aspect. It aliases to 'master' using the existing publish action. Effectively no change yet.
  • allow the new process to be manually run to test, understand, gain confidence in the mechanism that is in the commit.

At this time the creation of the 'latest' pseudo-tag does occur inside the GHA workflow build-and-deploy.yml
I think this could be moved to a sharable action in bin/source.sh so it can be used with the existing workflow easier, but I'd be changing it blind (untested).

With the link check:

  • It can be run in CHECK mode (SKIP, CHECK, FAILON) are the 3 modes. This way it will report issues that could fail but not actually fail a GHA workflow.
  • To me linkcheck is only useful for a supervisor performing maintenance on the documentation, with the specific intention and capacity to fix broken links at that time. At all other times the linkcheck report is only advisory, useful to know but not blocking anything.
  • It maybe useful to run a linkcheck periodically (and separately) on its own, which is possible with an on schedule action and the workflow script inside this change-set. Maybe it is also possible to auto-generate an issue via a bot, with the details, but I think running it monthly so there was always a recent report in GHA to inspect is useful enough.
  • Any new link a contribution introduces I assume will be inspected during the review process. So it must at least work at that time and be found to be appropriate content for the project after review. I don't see linkcheck as a tool to manage concerns in this area.
  • Failing a link check should not be a road-block to accepting, processing, merging documentation updates. Doing anything else is advocating that we don't update our source of primary information because of what a 3rd party website did. We should seek to always update and use feedback from linkcheck asynchronously.
  • I see a contributor getting a green-clean GHA workflow pass back as important for the contributor to observe in their process. The contributor should not be too heavily involved in a failed outcome from linkcheck since the report it generates is not targeted at the contributor, for the reasons explains above.
  • I do not consider a single out-of-date URL link in the documentation to be a show stopper to the publishing of more useful information. It is more important to get better information out today, than it is to wait for a maintainer to separately fix a broken link from documentation committed years ago. I would be happy to publish update docs today containing a known broken link because link maintenance can't be done today.
  • Out of date links naturally happen over time and is a long term maintenance issue that is independent of all other tasks.

@andreasWallner
Copy link
Collaborator

After our discussion this lgtm

@dlmiles
Copy link
Contributor Author

dlmiles commented May 31, 2023

RED build failure marker due to: new linkcheck issue

(SpinalHDL/Introduction/Projects using SpinalHDL: line 20) broken https://datenlord.github.io/en/home.html - 404 Client Error: Not Found for url: https://datenlord.github.io/en/home.html

However this linkcheck failure and the page has nothing to do with this change-set and should be fixed/amended in a separate PR. This is setup in another PR now.

@dlmiles dlmiles mentioned this pull request May 31, 2023
dlmiles added 17 commits May 31, 2023 14:15
…n/**)

This workaround exists because Sphinx wants to download the
TravisCI SVG badge (due to a URL listed in older documentation content),
but that URL is returning HTML content (maybe under HTTP/404) and the
Sphinx image processing terminates the documentation generation due
to an SVG parse error of the HTML document.

Ensure to use: source bin/setup_env.sh

Original patch split to help with cherry-pick into other branches.
…nf.py)

This workaround exists because Sphinx wants to download the
TravisCI SVG badge (due to a URL listed in older documentation content),
but that URL is returning HTML content (maybe under HTTP/404) and the
Sphinx image processing terminates the documentation generation due
to an SVG parse error of the HTML document.

Ensure to use: source bin/setup_env.sh

Original patch split to help with cherry-pick into other branches.
…ckerfile)

This workaround exists because Sphinx wants to download the
TravisCI SVG badge (due to a URL listed in older documentation content),
but that URL is returning HTML content (maybe under HTTP/404) and the
Sphinx image processing terminates the documentation generation due
to an SVG parse error of the HTML document.

Ensure to use: source bin/setup_env.sh

Original patch split to help with cherry-pick into other branches.
Would be nicer to have this format hosted (viewer can Ctrl-F to find)
Viewing old version warning dialog, to better inform the user if there is
 an updated documentation version.   Dismiss button supresses
 representation of dialog for 24 hours.
Adding MIT license GH ribbon chrome, with links to GH source of page
 in view. TXT files created to document source/version/component license.
Meta element robots added, repository clone and deployments are by
 default noindex/nofollow.
Minor CSS blance changes.
CSS tested for narrow forms (mobile/tablet).
Attempt towards a single CI environment for forking, building, deploying
that works equally for testing/forks as well as production publishing.

For me I get an auto-build when I push a branch called 'test' on my fork.
This will be the same for anyone forking the project to improve it, they
can manually run 'workflow_dispatch' or get an autobuild pushing to 'test'.

It is still possible to get a quick build like before using
workflow_dispatch selecting only BUILD_SINGLE_DOC and DEPLOY_ENV_TEST.

For production it should be possible to manually run 'workflow_dispatch'
from GH actions selecting the 3 options with "(live)" in the description.
BUILD_MULTI_DOC, FAILON_LINKCHECK, DEPLOY_ENV_LIVE.

The linkcheck is optional and options exist to allow bypass if there is
an temporary issue with a 3rd party website at the time of building.

Due to the way GH actions works, this commit needs to hit the master branch
before it shows up the controls for workflow_dispatch.

It is also advisable to setup an 'on schedule' to run once a month,
because GH pages expire after 3 months which removes all the
documentation.  So there is a step to control build settings for this
run but no entry at the top of file to setup a trigger.  Until the
project is happy it can run on auto-pilot.

The build process deals with:
  latest pseudo-tag for the latest version
  meta robots: to try to make test builds invisible to SEO
  linkcheck can be skipped, or enforced, or report only (non fatal)
  validating deployment to production have expected options
I was seeing a chmod getting errors, although they may not terminate
the JamesIves/github-pages-deploy-action@v4 action.
We are using an incorrect way to add additional CSS files.

This is causing a number of problems.

 * Unable to update sphinx-rtd-theme, due to no CSS styling
 * Issue#146 Copy button for code (button not displayed as expected)
 * Extensions that contribute CSS not working

The documentation for Spinix confirming this commit uses the correct method:

https://docs.readthedocs.io/en/stable/guides/adding-custom-css.html#how-to-add-custom-css-or-javascript-to-sphinx-documentation
This bump is working now since fixing the html_js_files/html_css_files
in the configuration.
@dlmiles
Copy link
Contributor Author

dlmiles commented May 31, 2023

Just rebase onto master 61d4fd8

git rebase --onto master HEAD~17 HEAD
git push github dlm-202307 -f
git log -n1 --oneline master
61d4fd8 (upstream/master, upstream/HEAD, master) Merge pull request #182 from dlmiles/dlm-linkfix1

To confirm rebuild all green from GHA then we all good to merge.

@andreasWallner
Copy link
Collaborator

This looks good to me
@wifasoi Is there anything you want to add, otherwise I'd propose we merge this

@wifasoi
Copy link
Collaborator

wifasoi commented Jun 4, 2023

Sorry for my late response. LGTM and merging (I'll be here to monitor the pipeline if it does funny things)

@wifasoi wifasoi merged commit e4b6eea into SpinalHDL:master Jun 4, 2023
3 checks passed
@andreasWallner
Copy link
Collaborator

@dlmiles Thanks!

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.

None yet

4 participants