Closed plowsof closed 2 years ago
Built without sensitive environment variables
Name | Link |
---|---|
Latest commit | 51ff4aa146d903701ac053e337823c44e8d0bcc1 |
Latest deploy log | https://app.netlify.com/sites/barolo-time-757cf9/deploys/629a96c5859fab000818a8da |
Deploy Preview | https://deploy-preview-1973--barolo-time-757cf9.netlify.app |
Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site settings.
Thanks for the detailed reply @erciccione (more than few concerns :D )
Do we really need a script for this? During review is quite trivial to see if an entry is added in alphabetical order and if it's not you can just ask the contributor to change the order. It would be a 1 minute fix. This is the approach we used for the old merchants page and we never had problems with it.
We don't need any checks, all problems can be fixed with time.
A python script is inconsistent with the other scripts we use and that integrate with jekyll. See the _plugins folder. If we are adding scripts, better stick with Ruby.
Breaking a build because of incorrectly ordering a page might be discouraging for a contributor, rather than pushing it to github and finding out then (where people can help / link them to a 'easy-guide' script)
Few user guides are usually added to getmonero (last addition was 9 months ago). If we are being very optimistic and assuming a drastic increase of new user guides (which is unlikely), this script + workflow will run maybe 2-3 times a year.
After reading the 'how to add a user-guide' docs, i understand why not many guides are added. With my script you just point it to a github URL. I then made 2 drafts for guides. i2p zero gui with monero gui and also pruned node with monero gui. They are 1 click and ready for a PR.
The FAQ is another story, the docs literally say 'a guide would be too complicated'. Also have a 1 click deployment. (that auto arranges in alphabetic order for #1969
I don't think it's needed (see point above), but all this could be replaced by a ruby plugin that checks if the list is in alphabetical order before building the website, and automatically sort it if it's not.
Interesting idea. Automatic sorting is the most ideal solution. But read your last and first point, an external check would be easier to maintain as you can just delete it from the workflow when things change / tell the contributor to fix it / use the script.
(i doubt im going to change your mind, but i just wanted to pay you the same respect of taking the time to comment, and i learned about some of the possibilities of workflows)
Do we really need a script for this? During review is quite trivial to see if an entry is added in alphabetical order and if it's not you can just ask the contributor to change the order.
But why not automate it so that it can't be missed during a review process? It doesn't cost us anything and it doesn't clutter up the repo as it's all contained in .github
. I don't see any downside.
A python script is inconsistent with the other scripts we use and that integrate with jekyll.
Using python for this is sensible choice, we have something similar in the GUI repo even though we don't use python for the GUI. The script is part of a Github workflows process, not the website itself.
If this were something that would be executed during the website build process than I would agree with you, but that's not the case here.
Edit: In general I think it's good to have all "rules" that have to be manually checked during the review process automated, similar to how we currently have workflows for a new release that checks hashes.
Proposed to revert this pr in #2008
Test will fail on push/pr of the user-guide index page if its not in alphabetic order / succeed if it is. will help repo maintainer with #1967 . (there is an easy-guide.py script to help with creating a guide / auto alphabetic ordering too, not in this PR)
help for reviewer : im new to workflows so 'the names for jobs' may be wrong / vague
To test, a PR can be made to this repo where the same script is being used (the index is currently in alphabetic order)