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

Issue template form — alternate take #4308

Merged
merged 7 commits into from
Nov 29, 2023
Merged

Conversation

joepeeples
Copy link
Contributor

@joepeeples joepeeples commented Nov 21, 2023

Contributes to #4229 with an alternative version of #4301. This version combines some fields to streamline the flow, and makes a few other assorted adjustments.

Preview:
https://github.com/elastic/security-docs/blob/issue-template-form-redux/.github/ISSUE_TEMPLATE/internal-docs-request.yaml


Summary of edits

  • Consolidate:

    • Resources + Point of contact + Test environments - All are asking, where do we go to learn more about how this feature works?
    • Prereqs, privs + Feature flags - Both are asking, what do you need to enable this feature?
  • Background & resources: Changed the placeholder text into default text, so it persists when you click on the field. I really don't want people to forget to include any of these bits of info, which they might if the text disappears when they start typing.

  • Remove: Acceptance Test Criteria. This section has always been confusing to me, and historically I don't think people follow this format when creating issues. It also seems like a misnomer to call it "test criteria" since we're documenting, not testing. I think this section was asking for the feature's typical workflows, which could be part of the main Description's prompt.

  • Release timelines: We need to know both the Stack release and the serverless release timelines, so added a field just for serverless release.

Copy link

Documentation previews:

This comment was marked as resolved.

@joepeeples joepeeples self-assigned this Nov 21, 2023
@joepeeples joepeeples marked this pull request as ready for review November 21, 2023 20:57
@joepeeples joepeeples requested a review from a team as a code owner November 21, 2023 20:57
Copy link
Contributor

@nastasha-solomon nastasha-solomon left a comment

Choose a reason for hiding this comment

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

Just left two comments for your consideration. Thanks for setting this up, @joepeeples!

- type: markdown
attributes:
value: |
Hello! This form will create an issue that the Security docs team will triage and prioritize. Please do not add any labels to your issue — we'll take care of that.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the DE team is the only one that needs the extra/custom labels so they can track doc issues on their project board. One option for reducing the increasingly large label load is to agree on a set of labels that we can both use for tracking purposes.

Copy link
Contributor Author

@joepeeples joepeeples Nov 28, 2023

Choose a reason for hiding this comment

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

I'm actually thinking maybe we just omit the sentence about labels. That was originally copied from the Obs docs team's form, where they're using automation to auto-label issues based on responses throughout the form (and maybe manually adding labels interferes with that?).

We're not planning to implement that, at least not for our first iteration, so we'll end up manually adding and adjusting labels regardless. I don't see why we should discourage anyone if they want to try to help us label an issue. @jmikell821 WDYT?

Why: This feature will make X, Y, and Z easier for the user.
How: The user navigates to *Foo* → *Bar*, then clicks the *Wow* button.
validations:
required: true
Copy link
Contributor

@nastasha-solomon nastasha-solomon Nov 28, 2023

Choose a reason for hiding this comment

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

Would be a good place to ask whether there's an API doc impact?

I'm also still unsure whether we should put a checkbox here, or add another text field. I'm leaning towards a text field so devs have the opportunity to add more detail, if they have more to share. @natasha-moore-elastic what are your thoughts here?

Suggested change
required: true
required: true
- type: textarea
id: description
attributes:
label: API docs impact
description: Describe what needs to be documented and provide example responses. What parameters and endpoints need to be documented?
placeholder: |
What: We're introducing new endpoint A.
Why: Users can send a request to endpoint A to use feature A.
How: The user provides the specific information in the request
validations:
required: true

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, a text field is more flexible for additional info. Minor edit to suggestion above: label: API docs impact

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, once I committed the suggestion and looked at the preview, it seems like maybe this should go lower in the order of fields, maybe between "Feature differences" and "Prerequisites...." IMO it kind of sticks out toward the top. It's a required field so the submitter can't overlook it even if it's lower.

Copy link
Contributor Author

@joepeeples joepeeples Nov 28, 2023

Choose a reason for hiding this comment

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

Moved in 5dc55c8 to see how it looks in the preview; we can revert or relocate elsewhere if needed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree with having a text field rather than a checkbox so that more info can be provided.

I can't seem to add a suggestion for line 24. I'd maybe update the description to something slightly more specific, like:
Provide endpoint and parameter descriptions, and requests and response examples.

Copy link
Contributor Author

@joepeeples joepeeples Nov 29, 2023

Choose a reason for hiding this comment

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

I think you can't comment on the suggestion above because I already committed it, then moved it to a different part of the file. The description for API docs is now line 82; you should be able to comment on that in the diff tab, @natasha-moore-elastic.

@nastasha-solomon nastasha-solomon requested a review from a team November 28, 2023 19:58
benironside
benironside previously approved these changes Nov 29, 2023
Copy link
Contributor

@benironside benironside left a comment

Choose a reason for hiding this comment

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

LGTM, just one super minor nit to add to the existing discussion

.github/ISSUE_TEMPLATE/internal-docs-request.yaml Outdated Show resolved Hide resolved
Co-authored-by: natasha-moore-elastic <[email protected]>
Co-authored-by: Benjamin Ironside Goldstein <[email protected]>
@joepeeples joepeeples merged commit 2bfd615 into main Nov 29, 2023
4 checks passed
@joepeeples joepeeples deleted the issue-template-form-redux branch November 29, 2023 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants