-
-
Notifications
You must be signed in to change notification settings - Fork 777
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
Added title to suggest-a-feature-or-new-content-for-hackforla-org.md issue template #7564
Added title to suggest-a-feature-or-new-content-for-hackforla-org.md issue template #7564
Conversation
…suggestion-form-update-2293
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.
|
Availability: 5pm-9pm M-F |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @mmcclanahan Thanks for working on this atypical issue. I can see that the original issue said to submit the PR with "Partial work for # 2293", most likely so the issue does not close when your PR is merged. This is why two workflows did not run correctly- but this is OK for this issue. Some notes:
- Because the description does not say "Fixes #..." the workflow to transfer labels is not going to run. Please add the same labels from 2293 to this issue
- You did substitute the correct text as shown on the issue. However, when we were writing the issue we should have caught that the text format is not consistent. I changed the formatting on the issue- would you please replace the text from the original with the new text?
- Per 2293, please add the
### Dependency ...waiting to see...
text at the top of 2293. (If you received different instructions from someone, please note that on the PR)
Besides these items, it appears that you satisfied the first steps of the original issue. Your branches are good, you reference the orig. issue, you explain what you did and why. Great job
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @mmcclanahan, thanks for taking on this issue!
Things you did well:
- branch name & pr reference issue being addressed
- Description and reason for changes are clear
- Title give a good idea of what the PR does
Things to change:
- PR does not contain labels. This makes it difficult to find the PR in the future.
- Issue calls for an edit to the Issue text. Add the dependency so we know the issue has per-requisites before we can move forward.
Once the changes are made ask for a re-review and I'd be happy to approve it. Keep up the good work!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes look good but the checks wont pass unless you change the Partial work
to fixes
Also you should provide the labels so we know if this is related to the back end or front end.
Keep it up! Please let us know if you need any help. @mmcclanahan
Thank you @t-will-gillis @FamousHero and @codyyjxn for the review and help! Please let me know if you catch anything else. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thank you for making the changes!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @mmcclanahan Thanks for making the requested changes/additions - looks great!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great Job! @mmcclanahan Thank you for taking on this issue.
Partial work for #2293
What changes did you make?
title: ''
withtitle: suggest a feature or new content for the hackforla.org website
Why did you make the changes (we will use this info to test)?
Screenshots of Proposed Changes To The Website (if any, please do not include screenshots of code changes)