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

Improve/Standardize alias #3011

Merged
merged 13 commits into from
Nov 26, 2024
Merged

Conversation

thejcannon
Copy link
Contributor

@thejcannon thejcannon commented Nov 24, 2024

  • key has aliases identifier and id (however I suspect y'all don't want to advertise the latter due to confusion with BUILDKITE_STEP_ID (not being the id)) so make sure identifier is listed as an alias
  • Document the "not-UUID-like" restriction on all keys
  • Add missing key field to Trigger and Wait step docs

@github-actions github-actions bot added the pipelines Pull requests that update content related to Pipelines label Nov 24, 2024
@@ -102,8 +102,9 @@ Optional attributes:
<tr>
<td><code>key</code></td>
<td>
<p>A unique string to identify the block step.</p>
<p>A unique string to identify the block step.<br>Keys can not have the same pattern as a UUID (<code>xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx</code>).</p>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure why bloack_step.md uses <p> for all the table cells, but none of the others do 🤔

Copy link
Contributor

@gilesgas gilesgas Nov 25, 2024

Choose a reason for hiding this comment

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

Good question @thejcannon , and thanks for pointing this out. I'm not sure why this is the case either.

I'll remove these <p></p> elements from this table then; this also appears to affect about 1/2 the HTML tables on this page.

@@ -118,9 +119,9 @@ Optional attributes:
<tr>
<td><code>key</code></td>
<td>
A unique string to identify the step. The value is available in the <code>BUILDKITE_STEP_KEY</code> <a href="/docs/pipelines/environment-variables">environment variable</a>.<br>
A unique string to identify the command step. The value is available in the <code>BUILDKITE_STEP_KEY</code> <a href="/docs/pipelines/environment-variables">environment variable</a>.<br>
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks!

@gilesgas
Copy link
Contributor

gilesgas commented Nov 25, 2024

Thanks very much for this PR update @thejcannon ! 🎉

These consistency updates are much appreciated! As mentioned above, I've removed the unnecessary <p></p> tag pairs and have added line breaks in the source after <br> (which I converted to <br/>) tags to improve the readability of this content in source.

Add missing key field to Trigger and Input step docs

I gather you meant Wait (instead of Input) here.

Anyway, while I'm pretty sure these are correct, I'll ask a colleague to provide a 2nd opinion first, and then'll merge these in! Thanks again

@thejcannon
Copy link
Contributor Author

These consistency updates are much appreciated! As mentioned above, I've removed the unnecessary

tag pairs and have added line breaks in the source after
(which I converted to
) tags to improve the readability of this content in source.

Hmm I'm not seeing where you did this. Did you mean to do it on my branch? Or did you do this on main?

I gather you meant Wait (instead of Input) here.

Yup! Updated!

@gilesgas
Copy link
Contributor

gilesgas commented Nov 25, 2024

Thanks for your replies!

Hmm I'm not seeing where you did this. Did you mean to do it on my branch? Or did you do this on main?

They've been done (along with the removal of the unnecessary <p></p> tags) in a separate branch of a mirror of this docs repo. Once your PR is merged in, you'll see these commits pop up on main in this docs repo.

@gilesgas
Copy link
Contributor

gilesgas commented Nov 26, 2024

FYI @thejcannon , please don't worry about the merge conflicts in this working branch of yours.

We've just implemented some major restructuring of most of the Pipeline docs pages to make them follow a consistent URL pattern. These changes are now in the main branch of this repo and live in the Buildkite Docs. Therefore, the merge conflicts you see here have been resolved in the branch of our mirrored docs repo.

EDIT: I just wrote this comment in the event that you received a notification about merge conflicts. Having said that, this was only a fleeting state and this PR was merged without issue (along with the additional commits from the mirrored branch).

@buildkite-systems buildkite-systems merged commit c346d67 into buildkite:main Nov 26, 2024
1 check passed
@thejcannon thejcannon deleted the patch-1 branch November 26, 2024 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pipelines Pull requests that update content related to Pipelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants