ethereum / EIP-Bot

A collection of bots that make life easier on editors
Creative Commons Zero v1.0 Universal
41 stars 37 forks source link

EIP bot must error when multiple files are in EIPs asset folder but e.g. only single file is linked in specification document #93

Closed TimDaub closed 2 years ago

TimDaub commented 2 years ago

The EIP bot must check the following:

If an EIP champion checks in multiple files in the asset directory, the EIP bot must check if all of those files are listed in a table in the specification document. Elsewise if e.g. only one file is checked in, the EIP bot must check if this one file is listed in the specification document.

In case e.g. a file is checked into the assets directory but not listed in the specification document, then the EIP bot must fail/throw to indicate to the user that they have to link all documents checked-into the assets folder.

MicahZoltu commented 2 years ago

This isn't a bot issue, it has to do with the fact that neither GitHub nor Jekyll markdown renderers (I believe) render folders, they only support linking to files. For EIPs, the current best option is to either create a manual table of contents markdown file and link to that in your EIP, or just put the table of contents into the EIP itself.

Generally speaking, we try to target the lowest common denominator for markdown rendering so the contents of this repository are maximally compatible with all markdown rendering systems. In this case, that means no support for folders.

TimDaub commented 2 years ago

then IMO tests should be created that check if all files in the assets folder are linked in the EIP. I shipped a folder structure but only linked a single file, only to find later that it lead to bad experiences with people downloading the reference implementation. I fixed it now, but it would have been nice had there been a warning/check.

MicahZoltu commented 2 years ago

If you want to update the Title and description of this issue to reflect that suggestion to the bot we can re-open it. I agree that would be a great addition.

TimDaub commented 2 years ago

OK, changed the title and edited the original post.

Pandapip1 commented 2 years ago

I think this would best be done in the renderer by adding an index.html table of contents in every folder and sub-folder of assets

MicahZoltu commented 2 years ago

I think this would best be done in the renderer by adding an index.html table of contents in every folder and sub-folder of assets

We have tried pretty hard in the past to ensure that EIPs repository renders in the lowest common denominator rendering system. Any tweaks we have made to Jekyll layouts are purely visual, and not functional. If we allowed people to link to directories and this only worked in the Jekyll rendered site, then any other rendering engine used would break and links would not work properly.

Pandapip1 commented 2 years ago

This best seems suited as an eipw issue.

Pandapip1 commented 2 years ago

Superseded by https://github.com/ethereum/eipw/issues/30