-
-
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
Update center.yml #3260
Update center.yml #3260
Conversation
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.
|
Review ETA: 7 PM 6/20/22 |
Availability: 1 hour |
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 @troywzhu, great work! I can see that you changed content: icon to content-type: image and removed the unnecessary line of code. Thank you!
Availability: 1 hour |
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.
It looks like the pull request has not been done from the correct branch.
From https://github.com/hackforla/website/wiki/How-to-Review-Pull-Requests:
"The commit from must be from the collaborator who opened the pulled request"
As such, the commit should be from "troywzhu:troywzhu-patch-3" (ie, from troywzhu's own account rather than a branch within hackforla).
Instead it looks like both the to and from branches are within hackforla: "troywzhu wants to merge 1 commit into gh-pages from troywzhu-patch-3"
The "from" branch should be from troywzhu's own account. Please redo the pull request with the correct branch.
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.
@troywzhu good work!!. I see that you changed content: icon to content-type: image and removed the unnecessary line of code.
However, you didn't seem to follow the correct branch naming convention for your issue
Note:
You are to choose a branch name that:
-relates to the issue (No spaces!)
-includes the issue number
-The format should be such that, the words are a brief description of the issue that will make sense at a glance to someone unfamiliar with the issue.
as opposed to using troywzhu-patch-3.
I hope you can make this correction in your subsequent issues.
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.
@troywzhu Good job, you condensed the code so that it now says content-type:image. Like other reviewers said, you did not follow the Branch Naming Procedure, so the branch name is not clear. You need to follow the correct branch to and from naming procedure, as others have listed above.
While the comments above are correct, I don't think it is possible to change the branch name. I am going to merge this request because the work is done correctly, and the author does not appear to be active with Hfla at the moment. @troywzhu For your reference, a better branch name would be something like:
See: Section 2.7.b of Contributing.md |
Fixes #2805
What changes did you make and why did you make them ?