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

Update blank issue form with dependency #3717

Conversation

Adastros
Copy link
Member

Fixes #2904

What changes did you make and why did you make them ?

The blank issue form with dependency (blank-issue-form.yml) was updated to move the dependency form fields from the bottom of the form to the top of the form. In addition, the name and description of the form in the yml file were updated to clarify what the form is for. This was performed to make it clear that the form is for issues with dependencies. This will make it easier and quicker to pick an issue template and write an issue.

Screenshots of Proposed Changes Of The Issue Form

Visuals before changes are applied

Blank Form with Dependency Issue Selection View - Before Update

Blank Form with Dependency - Form View - Before Update

Visuals after changes are applied

Blank Form with Dependency Issue Selection View - After Update

Blank Form with Dependency - Form View - After Update

The blank issue form (blank-issue-form.yml) was updated to move the
dependency form fields from the bottom of the form to the top of the form.

In addition, the name and description of the form in the ymal file were
updated to clarify what the form is for.
@github-actions
Copy link

Want to review this pull request? Take a look at this documentation for a step by step guide!

From your project repository, check out a new branch and test the changes.

git checkout -b Adastros-update-blank-issue-form-with-dependency-2904 gh-pages
git pull https://github.com/Adastros/website.git update-blank-issue-form-with-dependency-2904

@github-actions github-actions bot added Feature: Administrative Administrative chores etc. role: front end Tasks for front end developers size: 0.5pt Can be done in 3 hours or less Complexity: Small Take this type of issues after the successful merge of your second good first issue Status: Updated No blockers and update is ready for review labels Nov 14, 2022
@t-will-gillis t-will-gillis self-requested a review November 14, 2022 18:57
@MattPereira MattPereira self-requested a review November 14, 2022 21:39
@MattPereira
Copy link
Contributor

Review ETA: End of Day 11/14/22
Availability: 8 - 10 PM

@t-will-gillis
Copy link
Member

Review ETA: 11/14/22
Available: 9-10 pm 11/14

Copy link
Member

@t-will-gillis t-will-gillis left a comment

Choose a reason for hiding this comment

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

Hi Adastros- Great job! It appears that you have made the changes to blank-issue-form.yml just as asked for in issue #2904. These changes are testable and appear to function as intended with no unexpected side effects. So again looks great on your end!

I do have some questions/ comments: first whether the file name should change to blank-issue-form-with-dependency.yml , and second whether it is useful to have a dropdown asking if there is a dependency (since the answer should be 'yes' if you are using this form). HOWEVER these questions/comments were not part of the scope of this issue, so I will not 'request changes' on this issue.

@jyaymie jyaymie self-requested a review November 15, 2022 05:45
@jyaymie
Copy link
Member

jyaymie commented Nov 15, 2022

Review ETA: 11/14/22 11:30pm
Availability: 11/14/22 Now - 11:30pm

@Adastros
Copy link
Member Author

Hi @t-will-gillis, I messaged @JessicaLucindaCheng about her thoughts on renaming the file yesterday. She agrees on changing the name of the file to blank-issue-form-with-dependency.yml. I'll create an issue to address it. In regards to the dropdown, I think that removing it would be beneficial since by default the answer should be yes if the form is chosen. I also messaged @JessicaLucindaCheng for her thoughts as well.

Copy link
Member

@jyaymie jyaymie left a comment

Choose a reason for hiding this comment

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

This looks great, @Adastros! I appreciated the red boxes in your screenshots that clearly indicated the visible changes to expect. @t-will-gillis made excellent points on which you were quick to take action. Awesome!

Copy link
Contributor

@MattPereira MattPereira left a comment

Choose a reason for hiding this comment

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

Good work on this one @Adastros!

You flawlessly completed all of the action items from the issue, and it looks like you're doing a commendable job communicating with the issue creator about renaming the file. Great job addressing @t-will-gillis thoughtful suggestions. You might also consider changing the dependency-explanation textarea to be required if Jessica decides you should remove the dependency yes/no dropdown.

Keep up the good work! 👍

@arpitapandya arpitapandya merged commit 08beb9b into hackforla:gh-pages Nov 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Complexity: Small Take this type of issues after the successful merge of your second good first issue Feature: Administrative Administrative chores etc. role: front end Tasks for front end developers size: 0.5pt Can be done in 3 hours or less Status: Updated No blockers and update is ready for review
Projects
Development

Successfully merging this pull request may close these issues.

Update blank issue form with dependency
5 participants