Skip to content

[feat] Remove the state column, replace with a badge.#244

Merged
lukesneeringer merged 2 commits into
masterfrom
reviewing-badge
Aug 15, 2019
Merged

[feat] Remove the state column, replace with a badge.#244
lukesneeringer merged 2 commits into
masterfrom
reviewing-badge

Conversation

@lukesneeringer
Copy link
Copy Markdown
Contributor

This PR removes the "state" column from the general AIP index and replaces it with a "Reviewing" or "Draft" badge for AIPs in those states (and removes other states from the list entirely).

This change only affects the "general" page to avoid needing everyone from every team to approve.
After this is merged, I will send individual PRs to each team to migrate their indexes.

This PR removes the "state" column from the
general AIP index and replaces it with a
"Reviewing" or "Draft" badge for AIPs in those
states (and removes other states from the list
entirely).

This change only affects the "general" page to
avoid needing everyone from every team to approve.
After this is merged, I will send individual PRs
to each team to migrate their indexes.
@lukesneeringer lukesneeringer requested review from a team August 14, 2019 17:18
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Aug 14, 2019
@lukesneeringer
Copy link
Copy Markdown
Contributor Author

aip-badge

{%- for p in site.pages %}
<!-- prettier-ignore -->
{%- if p.aip and p.aip.id >= include.start and p.aip.id <= include.end and
'approved,reviewing,draft' contains p.aip.state %}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't have an in operator ?

Copy link
Copy Markdown
Contributor Author

@lukesneeringer lukesneeringer Aug 14, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope. Liquid is silly that way.

Comment thread aip/general.md
{% endfor %}

<!-- prettier-ignore-end -->
{% include aip-listing.html start=100 end=119 %}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are the start/end parameters doing something specific? Can we comment this ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, they tell the included file which AIPs to render. So this means that the table will include any AIPs between 100 and 119, inclusive.

(This used to be hard-coded in the Markdown.)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a comment in the included file.

@lukesneeringer lukesneeringer merged commit b1a8371 into master Aug 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants