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.

Sign up for GitHub

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

Jump to bottom

Update Partner Text Under BallotNav Project #5572

Merged
merged 1 commit into from
Oct 3, 2023
Merged

Update Partner Text Under BallotNav Project #5572

merged 1 commit into from
Oct 3, 2023

Conversation

danielmshawn
Copy link
Member

@danielmshawn danielmshawn commented Sep 23, 2023 •
edited
Loading

Fixes #5410

What changes did you make?

  • Within _projects/ballot-nav.md, removed line "partner: Code for America, Other Brigades"
  • Replaced line with "partner: Code for San Jose, Code for Atlanta, Open Oakland"

Why did you make the changes (we will use this info to test)?

  • We need to remove mention of CfA from the list of partners on the Ballot Nav project

Screenshots of Proposed Changes Of The Website (if any, please do not screen shot code changes)

Visuals before changes are applied Screenshot 2023-09-14 at 5 53 27 PM
Visuals after changes are applied Screenshot 2023-09-14 at 6 14 33 PM

@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 danielmshawn-remove-cfa-ballot-nav-5410 gh-pages
git pull https://github.com/danielmshawn/website.git remove-cfa-ballot-nav-5410

@github-actions github-actions bot added good first issue Good for newcomers role: front end Tasks for front end developers role: back end/devOps Tasks for back-end developers P-Feature: Project Info and Page A project's detail page (e.g. https://www.hackforla.org/projects/100-automations) time sensitive Needs to be worked on by a particular timeframe size: 0.25pt Can be done in 0.5 to 1.5 hours labels Sep 23, 2023
@A-Wu5 A-Wu5 self-requested a review September 23, 2023 03:45
@A-Wu5
Copy link
Member

A-Wu5 commented Sep 23, 2023

Review ETA: 10 PM 9/22/23
Availability: 5-8 PM Monday

A-Wu5
A-Wu5 approved these changes Sep 23, 2023
View reviewed changes
Copy link
Member

@A-Wu5 A-Wu5 left a comment

Choose a reason for hiding this comment

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

Everything looks good to me! The changes meet the criteria stated in the issue. Nice job!

@k-rewd k-rewd requested review from k-rewd and removed request for k-rewd September 23, 2023 05:44
@k-rewd k-rewd assigned k-rewd and unassigned k-rewd Sep 23, 2023
@t-will-gillis t-will-gillis self-requested a review September 24, 2023 16:16
t-will-gillis
t-will-gillis requested changes Sep 24, 2023
View reviewed changes
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.

Hey @danielmshawn - Everything looks great- you have the correct branches, you are linking to the original issue, you are including relevant before and after screenshots, and your edits change only what needs to be changed.

One minor request: On the description for what you did and why: the "what" section is good because it briefly describes the changes. On the "why" section, what we are looking for here is the reason that the changes needed to be made. This can be copied and/or summarized from the original issue, for example: "We need to remove mention of CfA from the list of partners on the Ballot Nav project"

Otherwise, this is good to go- thanks!

@t-will-gillis
Copy link
Member

t-will-gillis commented Oct 1, 2023 •
edited
Loading

Hello @danielmshawn Checking in from last week- FYI when a PR has requested changes, it is blocked from being merged until either the requested changes are made, or a reason is given for why the requested changes should not be made.

Could you please make the requested changes? Please let one of us know if you would like to discuss. Thank you!

@danielmshawn
Copy link
Member Author

Apologies, @t-will-gillis! I misread. Requested changes to the "Why" have been made, and I'll make sure to put the correct info there from now on. Thank you!!

@t-will-gillis t-will-gillis self-requested a review October 3, 2023 18:33
t-will-gillis
t-will-gillis approved these changes Oct 3, 2023
View reviewed changes
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.

Hey @danielmshawn No problem! Thanks for making the change

@t-will-gillis t-will-gillis merged commit f273635 into hackforla:gh-pages Oct 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@A-Wu5 A-Wu5 A-Wu5 approved these changes

@t-will-gillis t-will-gillis t-will-gillis approved these changes

Assignees
No one assigned
Labels
good first issue Good for newcomers P-Feature: Project Info and Page A project's detail page (e.g. https://www.hackforla.org/projects/100-automations) role: back end/devOps Tasks for back-end developers role: front end Tasks for front end developers size: 0.25pt Can be done in 0.5 to 1.5 hours time sensitive Needs to be worked on by a particular timeframe
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

remove mention of CfA from Ballot Nav project partners
4 participants
@danielmshawn @A-Wu5 @t-will-gillis @k-rewd