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

Use a consistent format for roles table notes in API docs #2322

Merged
merged 2 commits into from
Jun 10, 2021

Conversation

will-gant
Copy link
Contributor

Some tables had notes in the first column, while others had a second 'notes' column that was separate.

Also adds missing table headers for roles tables that have notes (usually marking a role as 'Experimental') , as the absence of these is currently preventing the second column from rendering altogether.

  • I have reviewed the contributing guide

  • I have viewed, signed, and submitted the Contributor License Agreement

  • I have made this pull request to the main branch

  • I have run all the unit tests using bundle exec rake

  • I have run CF Acceptance Tests

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jun 9, 2021

CLA Signed

The committers are authorized under a signed CLA.

@will-gant will-gant force-pushed the docs-table-columns branch from c577c32 to 8afe469 Compare June 9, 2021 13:16
@sweinstein22
Copy link
Contributor

sweinstein22 commented Jun 9, 2021

Hi @will-gant , thanks for the PR! One minor thing, it looks like there's still a bit of a discrepancy in the changes - some have the heading 'Role' while others have the heading 'Roles'. I think 'Role' makes more sense, given that each row pertains to a single role rather than a set of roles. Would you mind adjusting the ones included in this PR so that they're consistent?

Additionally rebasing the branch against main should resolve the unit test failures that are being seen in the github actions

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jun 10, 2021

CLA Signed

The committers are authorized under a signed CLA.

@will-gant will-gant force-pushed the docs-table-columns branch from d0a9370 to f374a08 Compare June 10, 2021 08:17
@will-gant
Copy link
Contributor Author

will-gant commented Jun 10, 2021

Change made. Thanks for the feedback :)

Sorry, I'm new at this - I can't see which unit test failure you're referring to. I also have a separate open PR, and that one does have failing unit tests (which I'm looking at now). Were you perhaps referring to those?

Also adds missing table headers for roles tables that have notes, as the absence of these prevents the second column from rendering.
@will-gant will-gant force-pushed the docs-table-columns branch from f374a08 to 7898d43 Compare June 10, 2021 09:29
@will-gant
Copy link
Contributor Author

Ok I've just rebased this off of main

@sweinstein22
Copy link
Contributor

sweinstein22 commented Jun 10, 2021

Thanks for making those changes @will-gant!

To explain my unit tests comment - we have automated Github actions that run against PRs, which includes unit tests run against mysql and postgres. Generally we want all the checks passing before we'll merge in a PR. They were failing on this PR, although as this is just a docs change it was pretty clear the failures didn't have to do with your work. Just checked and it looks like rebasing off main on your other PR worked as well.

Along those same lines, I am seeing a rubocop and unit test failures on another PR you submitted, I'll comment there.

@sweinstein22 sweinstein22 merged commit 4dcc0d8 into cloudfoundry:main Jun 10, 2021
@will-gant
Copy link
Contributor Author

Thanks for explaining and merging!

@mkocher mkocher added the space-application-supporter https://github.com/cloudfoundry/cfar-proposals/issues/22 label Aug 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
space-application-supporter https://github.com/cloudfoundry/cfar-proposals/issues/22
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants