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

Add header row to support gfm for table formatting #3297

Merged
merged 1 commit into from
Feb 26, 2021

Conversation

njgheorghita
Copy link
Contributor

When opening a pull request to submit a new EIP, please use the suggested template: https://github.com/ethereum/EIPs/blob/master/eip-template.md

We have a GitHub bot that automatically merges some PRs. It will merge yours immediately if certain criteria are met:

  • The PR edits only existing draft PRs.
  • The build passes.
  • Your GitHub username or email address is listed in the 'author' header of all affected PRs, inside .
  • If matching on email address, the email address is the one publicly listed on your GitHub profile.

Adds an empty header row to tables enable table formatting with github flavored markdown.

@MicahZoltu
Copy link
Contributor

MicahZoltu commented Feb 26, 2021

If you know how, would you mind throwing this into the page somewhere (I don't even know if it is possible):

td:empty {
  visibility: hidden;
}

And see if that resolves the issue? (you may need to tweak the specific settings)

The tables are rendering now, but they have an empty row in them.

@eip-automerger eip-automerger merged commit dba974e into ethereum:master Feb 26, 2021
@MicahZoltu
Copy link
Contributor

Hmm, maybe that isn't possible.

@MicahZoltu
Copy link
Contributor

Maybe we can update the jekyll site at least so it hides empty rows, and just deal with it in GitHub markdown...

@njgheorghita
Copy link
Contributor Author

Yeah I'm not able to find some straightforward way to do this - imo it's not that offensive to the eyes in the github flavored markdown since the empty header column is rendered quite small

@MicahZoltu
Copy link
Contributor

I'll talk to the other editors to see what they want to do.

@MicahZoltu
Copy link
Contributor

Spoke with some other editors: Would it be possible to change these to lists instead of tables? e.g.:

* **Required**: If `name` is included.
* **Key**: `version`
* **Type**: String
  • Required: If name is included.
  • Key: version
  • Type: String

@njgheorghita
Copy link
Contributor Author

Yup, works for me! That should be suitable for all the tables afaik - I'll submit the update asap

@MicahZoltu
Copy link
Contributor

Tables with actual headers I think everyone is good with being a markdown table. It is just the headerless tables that are problematic.

Arachnid pushed a commit to Arachnid/EIPs that referenced this pull request Mar 6, 2021
Hi, I'm a bot! This change was automatically merged because:

 - It only modifies existing Draft, Review, or Last Call EIP(s)
 - The PR was approved or written by at least one author of each modified EIP
 - The build is passing
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.

3 participants