Remove "Express" Label from Civic Tech Jobs Page#8371
Remove "Express" Label from Civic Tech Jobs Page#8371ryanfkeller merged 2 commits intohackforla:gh-pagesfrom
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. |
myronchen-git
left a comment
There was a problem hiding this comment.
Thank you for working on this issue.
Done Correctly
- The merge from and to branches have been correctly set.
- The issue has been correctly linked.
- The pull request title is named properly.
- The whats and whys in the pull request description have been filled out.
- The CodeQL alerts have been correctly checked off.
- Screenshots accurately reflect before and after changes.
- Code changes have been correctly made.
- In the issue, you've correctly assigned yourself.
- In the issue, the project and its status has been correctly assigned.
Changes that need to be made
- In this pull request, please remove the note at the top, above "Fixes".
Optional recommendations
- In the issue, you didn't give your availability and ETA. You can put that in now for completeness, but you should get into the habit of giving them after you've assigned yourself, even if you are able to complete the work in a few hours.
|
Availability: I'm available this week between 1pm to 5pm on Friday, Oct. 17. |
|
@myronchen-git - Thank you for your review and notes. I have made the requested changes. |
myronchen-git
left a comment
There was a problem hiding this comment.
Thank you for making the changes.
Your availability and ETA are supposed to be in the originating issue, not here in your pull request.
|
Review ETA: 10/21/25 |
rleeoriginal
left a comment
There was a problem hiding this comment.
Thanks for working on this issue, @aymeviviana !
Things Done Well
- Pull request contains correct branch.
- Linked issue is properly added.
- Screenshots of changes are included.
- Source code changes are applicable and clean.
Suggestions
None.
|
Hi @aymeviviana, thank you for taking on this issue! The changes you made look good, but there is a git house-keeping complication -- a different update also changed the Civic Tech Jobs technology section, and now git doesn't know how to automatically deal with adding your changes (removing Express) and their changes (adding Node.JS). To resolve this, you'll want to "rebase" your changes on top of the lastest version of the git checkout remove-tech-express-8278 # make sure you're on your issue branch
git fetch upstream # update your local git repo's understanding of what's on github
git rebase upstream/gh-pages # perform the rebase so that your changes are effectively on the latest gh-pages
# ... you'll have to deal with the merge conflict manually, accepting the incoming change
# then once you're done...
git add _projects/civic-tech-jobs.md # add your conflict-fixed file
git rebase --continue # continue with the rebase after your successful fix
# ... if you're doing all this in the command line, it might open a VI window for you to add a message. You can just exit. If you're doing it in the VSCode editor (recommended) it should be no problem.
git push -f origin remove-tech-express-8278 # force push your update to your local fork (which updates this PR)
# force push is necessary because the history changedIf you're not that familiar with git, all of this can be a lot to deal with at first, but it gets easier and we are here to help! Please shoot me a Slack message if you need any help or have any questions. Thanks for taking a stab at this, git skills will take you far! Here's a handy reference on what rebasing is and how to resolve merge conflicts, if you need it: |
5e9e16e
1e7954f to
5e9e16e
Compare
|
Hi @ryanfkeller - Thank you so much for your very helpful, detailed instructions! I walked through the commands you provided, resolved the merge conflict and pushed my changes. Will this PR require two new approvals before it gets merged? |
|
@aymeviviana -- great work! Yes, sadly this reset the reviews and I'm not able to merge until we have 2 approvals... but I'll go ahead and approve myself and re-request from your other reviewers, and we'll get this closed out ASAP! |
ryanfkeller
left a comment
There was a problem hiding this comment.
Nice work making this change and dealing with the rebase! Approved!
myronchen-git
left a comment
There was a problem hiding this comment.
Hi Ayme.
I do not see any changes to the repository that adds back inNode.js. There was a recent change that removed it. Looking at the current civic-tech-jobs.md, there is no line containing Node.js. Looking at the Files changed tab of your pull request, the right-hand side shows that you are adding something new that wasn't there before.
Please make a change to your commit and remove Node.js.
|
Oof... apologies @aymeviviana, @myronchen-git is right! It looks like the merge conflict was from #8373, and was actually removing Node.JS, not adding it back in, and I made a mistake when looking through the git history. Thanks @myronchen-git for the catch here! Yes, @aymeviviana, please do simply remove Node.JS and push up that change, no need to rebase here. Thanks! |
|
Hi @ryanfkeller - for this change, which arguments do I need to pass to the |
|
@aymeviviana -- good question, after you've made and committed the change, I'd go with
You don't need to include |
Fixes #8278
What changes did you make?
Why did you make the changes (we will use this info to test)?
CodeQL Alerts
After the PR has been submitted and the resulting GitHub actions/checks have been completed, developers should check the PR for CodeQL alert annotations.
Check the PR's comments. If present on your PR, the CodeQL alert looks similar as shown
Please let us know that you have checked for CodeQL alerts. Please do not dismiss alerts.
Instructions for resolving CodeQL alerts
If CodeQL alert/annotations appear, refer to How to Resolve CodeQL alerts.
In general, CodeQL alerts should be resolved prior to PR reviews and merging
Screenshots of Proposed Changes To The Website (if any, please do not include screenshots of code changes)
Visuals before changes are applied
Visuals after changes are applied