-
-
Notifications
You must be signed in to change notification settings - Fork 794
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 Leader #5923
Added Leader #5923
Conversation
ETA: EOD 11/22 |
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 there @zip-dazie , this PR that you generated doesn't look usual, it is missing the labels, also no GHA(Github Actions) done on this one, plus the issue that you are originally assigned for doesn't seem to have action items
checked.
Also the screenshot in the PR description is also not shown
I think the best would be close this particular PR and create a new one.........
Review ETA: 6 PM 11/22/23 |
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 labels have been added, the before and after pictures are now visible. This branch is coming from the correct branch and going into gh-pages. There is a linked issue, it is correct and addressed in the PR by adding Lane's info line 25-31. The code looks clean, only one commit and only one file changed.
The changes were verified in www.hackforla.org/projects/expunge-assist and localhost:4000/projects/expunge-assist
Good job!!
The command below was used to check in my project repository.
git checkout -b zip-dazie-add-leader-5823 gh-pages
git pull https://github.com/zip-dazie/website.git add-leader-5823
Review ETA: 11/22/2023 |
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 @zip-dazie great job to the added changes requested for this page.
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 there @zip-dazie can you please remove the extra-spaces (new lines) you added on line 24, 32, 61
. Great effort in making your code highlighted for the reviewers but I guess it disrupts the code consistency/code style
of the file. Rest of the issue is cool.....
- the to and from branch is good
- the visuals are perfect now and are similar to the one in the local environment
- the labels are added manually and correctly
- the changes are as specified and are apt
Jus do the one thing and I'll approve it!!
@freaky4wrld good job catching the extra spaces. @zip-dazie please remove those extra spaces so we can merge this. |
04f8fcf
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 there , great work there all the work assigned to you in the issue is done correctly
- the to/from branch looks good
- the visual changes are completely applied in the local environment
- the changes specified is done correctly and are apt
- the issue linked is correct too
Great work there, I approve this PR.......
Fixes #5757
What changes did you make?
Added Lane Mitchel to Project Profile: Expunge Assist
Why did you make the changes (we will use this info to test)?
To keep project team info up to date.
Screenshots of Proposed Changes Of The Website (if any, please do not screen shot code changes)
Visuals before changes are applied
Visuals after changes are applied