Skip to content
This repository has been archived by the owner on Feb 9, 2023. It is now read-only.

[enhancement, priority low] Make bot automatically copyedit #55

Closed
Pandapip1 opened this issue Mar 10, 2022 · 14 comments
Closed

[enhancement, priority low] Make bot automatically copyedit #55

Pandapip1 opened this issue Mar 10, 2022 · 14 comments

Comments

@Pandapip1
Copy link
Member

Pandapip1 commented Mar 10, 2022

I suggest that @eth-bot automatically submit a review requesting changes when travis detects spelling errors, or the bot detects missing EIP header info, missing EIP sections, incorrect filenames, and the like. I would estimate that the average EIP author would save a dozen years per second.

This is probably low-priority, given the high complexity.

@Pandapip1
Copy link
Member Author

Note: The dozen years per second is likely an overestimate.

@alita-moore
Copy link
Contributor

alita-moore commented Mar 11, 2022

Agreed. Low priority for sure (due to high complexity)..

Haha a dozen years. Ambition is a virtue, afterall..

@Pandapip1
Copy link
Member Author

I'll make a list of things that I'd suggest the bot do:

  1. If the EIP number is missing, directly edit the file to set the EIP number to the PR number, automatically replace all instances of EIP-XXXX to the actual EIP number (including the PR title), and change the filename.
  2. If the EIP is in the title or description, directly remove it.
  3. If the title, description, discussions-to, type, or category are missing, add comments prompting the user to add them.
  4. If the created field is missing, fill it in with the current date.
  5. Automatically edit the requires field to include all EIPs referenced (assets folder, EIPs folder, text containing eip-XXXX, erc-XXXX, eipXXXX, or ercXXXX).
  6. Automatically add the CC0 waiver at the bottom if missing.
  7. Warn users of missing EIP sections.
  8. Point out probable spelling errors by adding suggestions.
  9. Warn about external links. Add a folder in the EIP's assets folder called references, and put a PDF of the linked documents in it.
  10. In the Specification section, suggest to replace all links to solidity files to inline versions.

More to be added.

@alita-moore
Copy link
Contributor

cc @MicahZoltu ^

@MicahZoltu
Copy link
Contributor

If the EIP number is missing, directly edit the file to set the EIP number to the PR number, automatically replace all instances of EIP-XXXX to the actual EIP number (including the PR title), and change the filename.

This should be two separate items I think. One to set the EIP number, and another to fix all incorrect references to other EIPs. Regarding setting the EIP number in the header, I'm a bit hesitant to automate this because it is already a source of people trying to game the system, and automating it I fear will make that problem worse. Assigning EIP numbers is probably the easiest part of the editor job, so not doing this feels like it may be better.

@MicahZoltu
Copy link
Contributor

I like the idea of crawling links and putting a PDF version of them in assets automatically, but it feels like a lot of work and I worry that most users won't want that in the end.

I'm generally in favor of most of those changes though. The value provided by each varies greatly, so for prioritization it may make sense to figure out which ones would be the most impactful.

@Pandapip1
Copy link
Member Author

Maybe editors can manually set the EIP number in the header, and then the bot updates it?

@JEAlfonsoP
Copy link
Contributor

Hi Guys,
Suggestions for Pandapip1 list:

  1. EIP number is and must be assigned by Editors. For this EIP 1 / EIP - template must be completed (full fitted) by author(s), if-not, EIP number is not assigned by editor (?).
    2, 3, 4, 6, 7 could be implemented by bot (assert..)
    5, 8, 9, 10 do not see the consensus from you guys in the comments. Needs further discussion/feedback.

@SamWilsn
Copy link

SamWilsn commented Apr 7, 2022

I think there are actually a couple different features mixed together in your list @Pandapip1. There are some fields which should (or can only be) set upon merge:

(1) Setting the EIP number
(4) Setting the created date
(11) Setting the last-call-deadline date to 14 days after merging

Then there are lints which can either be automatically be applied+committed, or the bot could leave a comment:

(2) Disallow text matching eip[-\s]*[0-9]+ and erc[-\s]*[0-9]+ in title and description.
(3) Require all mandatory preamble headers are present and in the correct order.
(5) Warn if referenced EIPs are not included in requires preamble header.
(6) Require CC0 waiver.
(7) Require mandatory EIP sections and disallow others.
(8) Warn on spelling errors.
(9) Warn about external links.
(10) Warn about solidity in assets directory.

Where WARN means make a comment but do not request changes; REQUIRE means make a comment and request changes if the condition isn't met; and DISALLOW means make a comment and request changes if the condition is met.


Note that I've reworded some of these, but hopefully maintained their intent (mostly).

As I've worded them, I would support implementing them all, in the following order:

Highest priority: (1), (4), and (11) because they are difficult to do as an editor who isn't also a contributor with push permissions.

Next highest priority: (3) and (7) because they should be relatively easy and are super annoying to check.

Finally, everything else 🤣

@gcolvin
Copy link

gcolvin commented Apr 13, 2022

I like the idea of crawling links and putting a PDF version of them in assets automatically, but it feels like a lot of work and I worry that most users won't want that in the end.

It might also be a violation of copyright. And the assets might be huge. And can all assets be converted to PDF?

@Pandapip1 Pandapip1 changed the title Make bot automatically copyedit [enhancement, priority low] Make bot automatically copyedit Apr 18, 2022
@JEAlfonsoP
Copy link
Contributor

Hi, could we agree / define a possible action plan for this issue:

1, 4, 11 (upon merge)
3 and 7
The rest..
and not (include / create) any PDF file (mentioned in Pandapip1 list as number 9)

@Pandapip1
Copy link
Member Author

Pandapip1 commented Apr 20, 2022

This issue should probably be split into many issues with different priorities.

I disagree. Making PDFs of web pages might still be useful.

I suggest the following:

  1. All external links are converted to PDFs.
  2. All the PDFs are put in a new folder, assets/references.
  3. The links are changed to IPFS multihashes.
  4. The Ethereum foundation runs an IPFS node that pins these files.

I'll make this a new issue in the EIP repository.

@JEAlfonsoP
Copy link
Contributor

That's a good idea. Remove PDF actions from this one and create a new issue (low priority ??) for it. in this way we can identify a definitive action plan for this issue.

@SamWilsn
Copy link

I've split this up into a dozen or so issues. Please feel free to close this issue, and continue discussion in the appropriate places.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants