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

Added header support for wiki tables #335

Merged
merged 9 commits into from Nov 25, 2019

Conversation

ryanvilbrandt
Copy link
Contributor

@ryanvilbrandt ryanvilbrandt commented Nov 21, 2019

Following the syntax used in Wikidot:

||~ Head1 ||~ Head2 ||
||  cell1 ||  cell2 ||

Also added indenting to match the format found when using the tables extra.

@nicholasserra
Copy link
Collaborator

Thanks for the PR! Looks like this is failing some tests. Check the CI build :)

@ryanvilbrandt
Copy link
Contributor Author

Thanks for the PR! Looks like this is failing some tests. Check the CI build :)

Looking into it now.

@ryanvilbrandt
Copy link
Contributor Author

Okay, I believe I've followed all the steps for contributing a PR:

  • It must pass PEP8.
  • It must include relevant test coverage.
  • Bug fixes must include a regression test that exercises the bug.
  • The entire test suite must pass.

However, I'm not sure how to handle The README and/or docs are updated accordingly. I assume you don't want me to update your wiki until this change is merged in. Should I include the text change in a comment here?

@nicholasserra
Copy link
Collaborator

Nope no wiki update needed (yet?). I'll take a look at this early next week, thanks!

@nicholasserra
Copy link
Collaborator

I noticed you had to add {"extras": ["tables"]} to the Tables test case. Seems tables were working originally without the extra being turned on. So does this PR break backwards compat of built in tables feature?

@ryanvilbrandt
Copy link
Contributor Author

Tables.text is under the "-knownfailure" flag, so it wasn't being tested prior to my change anyways. I ended up fixing it before realizing I should be running the tests with that flag so I don't have to fix a bunch of problems that aren't related to my change. However, that change still manages to get a test working, even if it's not tested by CI, so I decided to leave it in.

@nicholasserra nicholasserra merged commit 0908539 into trentm:master Nov 25, 2019
@nicholasserra
Copy link
Collaborator

Thank you and thanks for the clarification :)

@ryanvilbrandt
Copy link
Contributor Author

When do you want the Extras wiki updated? Now that the change is merged into master? Or after you push a new release to pypi?

@ryanvilbrandt ryanvilbrandt deleted the wiki-table-headers branch November 25, 2019 16:45
@nicholasserra
Copy link
Collaborator

Any time works. I'm not totally sure if that wiki is up to date in general. If you want to take a stab at it, please do, 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

2 participants