search

hackforla / website

Hack for LA's website
https://www.hackforla.org
GNU General Public License v2.0
324 stars 763 forks source link

ER: Find all the places that GitHub should be GitHub and fix #6161

Open ExperimentsInHonesty opened 9 months ago

ExperimentsInHonesty commented 9 months ago

Dependencies (Child Issues)

Emergent Requirement - Problem

GitHub is a company name and should only be in lowercase (github) when in a URL, variable or directory name (where the variables or directories are all lowercase).

Details

There are 180 files with some form of github in it.

Issue you discovered this emergent requirement in

Date discovered

2024-01-25

Did you have to do something temporarily

Who was involved

@ExperimentsInHonesty

What happens if this is not addressed

Resources

Examples of locations that could be changed

Example of locations that could be changed, but it might break some logic.

Examples of locations it must be changed

Recommended Action Items

Potential solutions [draft]

a1exanderklein commented 9 months ago

Hi! I'm a student from the University of Florida looking to make a first-time contribution for an assignment. Mind if I take a shot at this issue?

ExperimentsInHonesty commented 8 months ago

@a1exanderklein if you are interested in joining the org (free) and working on issues, please signup for onboarding https://www.hackforla.org/getting-started

github-actions[bot] commented 7 months ago

Hi @SteeRevo, thank you for taking up this issue! Hfla appreciates you :)

Do let fellow developers know about your:- i. Availability: (When are you available to work on the issue/answer questions other programmers might have about your issue?) ii. ETA: (When do you expect this issue to be completed?)

You're awesome!

P.S. - You may not take up another issue until this issue gets merged (or closed). Thanks again :)

SteeRevo commented 7 months ago

Availability: Weekends 3pm-12pm ETA: This upcoming Sunday night at earliest, at latest 3/10/2024

SteeRevo commented 7 months ago

Hi @ExperimentsInHonesty, do you still happen to have the list of instances to change? If not that's totally OK.

SteeRevo commented 7 months ago

Provide Update

  1. Progress: Completed updating all comments and some titles
  2. Blockers: Still sorting through which GitHubs need to be updated
  3. Availability: 3pm-12pm
  4. ETA: 3/10/24
github-actions[bot] commented 4 months ago

Hi @roslynwythe, thank you for taking up this issue! Hfla appreciates you :)

Do let fellow developers know about your:- i. Availability: (When are you available to work on the issue/answer questions other programmers might have about your issue?) ii. ETA: (When do you expect this issue to be completed?)

You're awesome!

P.S. - You may not take up another issue until this issue gets merged (or closed). Thanks again :)

roslynwythe commented 3 months ago

Template for updates without risk (good first issues), for example: https://github.com/hackforla/website/blob/3c7c90e384563e35f5535a2784e1bca8b9cd2639/_sass/components/_guides.scss#L249

Prerequisite

  1. Be a member of Hack for LA. (There are no fees to join). If you have not joined yet, please follow the steps on our Getting Started page and attend an onboarding session.
  2. Before you claim or start working on an issue, please make sure you have read our How to Contribute to Hack for LA Guide.

Overview

We need to replace instances of Github and github with GitHub so that we display the company name correctly.

Action Items

Resources

Merge Team

roslynwythe commented 3 months ago

Template for updates with some risk, for example

Overview

We need to replace instances of Github and github with GitHub so that we display the company name correctly, and test the changes to ensure the behavior and appearance of any and all related webpages are unchanged.

Action Items

Resources

Merge Team

roslynwythe commented 3 months ago

@ExperimentsInHonesty please review both templates (above)

ExperimentsInHonesty commented 3 months ago

Replace this text

Devise, document and execute a test procedure to ensure that the behavior and appearance of all related webpages are unchanged.

with this

- [ ] Find all the files that are connected to this change.  (e.g., if its a yml or md file, what files or scripts use the data stored in that file).  And document briefly how they work together.
- [ ] Define where you expect the change to show up on the website (e.g., the footer of the website).  Can be on multiple pages
- [ ] Write a test procedure, in a comment, and then use it to test the change.  Include a link to this comment in your PR.
HackforLABot commented 1 month ago

Hi @daras-cu, thank you for taking up this issue! Hfla appreciates you :)

Do let fellow developers know about your:- i. Availability: (When are you available to work on the issue/answer questions other programmers might have about your issue?) ii. ETA: (When do you expect this issue to be completed?)

You're awesome!

P.S. - You may not take up another issue until this issue gets merged (or closed). Thanks again :)

daras-cu commented 1 month ago

Availability: Sat-Monday all day, other weekdays after 5 pm PST ETA: 9/9 EOD

daras-cu commented 1 month ago

Update:

I've created issues numbered 7413-7423, linked in the Dependencies list of the original PR comment above.

There are a few other files that need corrections to the GitHub spelling but I don't know if they fit into the two templates used here/if they need to be updated:

Also just a general question about creating good first issues, can most of them typically be labeled as both front end and back end? It seems that most are simple enough they could be worked on by either role.

t-will-gillis commented 1 month ago

Hi @daras-cu:

Just curious- do you have a 'master list' of which files need to be fixed?

I've created issues numbered 7413-7423, linked in the Dependencies list of the original PR comment above.

The issues you created look good and you listed them above- thanks for creating the issues! I have added the Ready for Prioritization label to each.

  • assets/js/wins.js and google-apps-scripts/wins-form-responses/Code.js: These files have instances of Github but it seems they are references to questions in the Google "Wins" form. I don't believe they can be updated without also changing them in the form but if someone with more knowledge of how the form works could weigh in on this I would appreciate it.
  • pages/wins/wins-share-form.html: Related, there are six lines that need to be corrected here but it doesn't seem like this page is used anymore as the Google form is used for "Wins" instead. Should I still create an issue/multiple issues for this file?

I do not have answers for the above two questions. @ExperimentsInHonesty @roslynwythe ?

  • CONTRIBUTING.md has 15 lines that need to be corrected. I am not sure if these should all be combined into one issue or separated.

I believe we would want multiple issues, however there are some logical groups that I think we would want to be a single, more complex issue, for example in Section 1.3 there are three Githubs in close proximity so make this a group correction??? and Complexity: Small???

  • github-actions/utils/hide-issue-comment.js has one instance but there's an open PR #7367 that replaces that line.

No longer applicable...

Also just a general question about creating good first issues, can most of them typically be labeled as both front end and back end? It seems that most are simple enough they could be worked on by either role.

I will let @ExperimentsInHonesty answer, but my vote would be to keep separating the issues appropriately but emphasize to devs that they can work on any issue they are comfortable with.

daras-cu commented 1 month ago

Hi @t-will-gillis, I did go through all the files from the Resources section (here) and made a spreadsheet of the ones that I thought needed to be changed to keep track: Google Sheet

Thank you for going through the issues I created so far. I'll go ahead and make the issues for CONTRIBUTING.md and group whichever instances make sense into Complexity: Small issues. I'll wait for feedback on the other files in question.

That makes sense re: the good first issues, I have been separating them out so will continue to do so.

Thanks again for your help!

daras-cu commented 1 month ago

Update:

I created issues numbered 7438-7443, linked in the original PR Dependencies list above. These all involve editing CONTRIBUTING.md. If there were multiple corrections to be made in the same section, I grouped them into one issue with Complexity: Small, otherwise I made a separate good first issue.

I think the only files remaining that need to be corrected for this ER are the files related to the Wins form mentioned in my earlier comment, so I'll wait for guidance on how to handle those.

ExperimentsInHonesty commented 1 month ago

@daras-cu I made all the contributing file changes have a small label because they are harder to test.

Update: I created issues numbered 7438-7443, linked in the original PR Dependencies list above. These all involve editing CONTRIBUTING.md. If there were multiple corrections to be made in the same section, I grouped them into one issue with Complexity: Small, otherwise I made a separate good first issue.

ExperimentsInHonesty commented 1 month ago

@daras-cu Yes, we label them as both front and back end if it's good first or small.

Also just a general question about creating good first issues, can most of them typically be labeled as both front end and back end? It seems that most are simple enough they could be worked on by either role.

ExperimentsInHonesty commented 1 month ago

@daras-cu

For the purposes of discussion, I have added all the details we are looking at for the GitHub wins page and form to this comment.

wins.js

wins.js

Lines we intended to change below https://github.com/hackforla/website/blob/a999c255a87d761e7579db1d76a6e0a0a3a05d39/assets/js/wins.js#L12 change ``` const github_url = "Github URL (optional)" ``` to ``` const github_url = "GitHub URL (optional)" ``` https://github.com/hackforla/website/blob/a999c255a87d761e7579db1d76a6e0a0a3a05d39/assets/js/wins.js#L13 change ``` const github_permission = "Could we use your Github profile picture next to your story?" ``` to ``` const github_permission = "Could we use your GitHub profile picture next to your story?" ``` https://github.com/hackforla/website/blob/a999c255a87d761e7579db1d76a6e0a0a3a05d39/assets/js/wins.js#L28C1-L28C74 change ``` "I increased the number of commits on my Github profile": `github.svg`, ``` to ``` "I increased the number of commits on my GitHub profile": `github.svg`, ``` https://github.com/hackforla/website/blob/a999c255a87d761e7579db1d76a6e0a0a3a05d39/assets/js/wins.js#L334 change ``` cloneCardTemplate.querySelector('.github-icon').alt = `Github profile for ${card[name]}`; ``` to ``` cloneCardTemplate.querySelector('.github-icon').alt = `GitHub profile for ${card[name]}`; ``` https://github.com/hackforla/website/blob/a999c255a87d761e7579db1d76a6e0a0a3a05d39/assets/js/wins.js#L509 change ``` makeIcon(data[i][github_url], overlayIcons, 'github-icon', '/assets/images/wins-page/icon-github-small.svg', 'Github profile for ' + data[i][name]); ``` to ``` makeIcon(data[i][github_url], overlayIcons, 'github-icon', '/assets/images/wins-page/icon-github-small.svg', 'GitHub profile for ' + data[i][name]); ```

Code.js

Any changes in this file (code.js) will have to also be made in the Google App Script attached to the spreadsheet

Lines we intended to change below https://github.com/hackforla/website/blob/50af24636cc916192b5dc7d06d36ed8fe72767c0/google-apps-scripts/wins-form-responses/Code.js#L147 Change ``` processedFormDataObject.set("GitHub", rawFormDataObject["Github URL (optional)"]) ``` to ``` processedFormDataObject.set("GitHub", rawFormDataObject["GitHub URL (optional)"]) ``` https://github.com/hackforla/website/blob/50af24636cc916192b5dc7d06d36ed8fe72767c0/google-apps-scripts/wins-form-responses/Code.js#L148 Change ``` processedFormDataObject.set("GitHub Picture Allowed ?", rawFormDataObject["Could we use your Github profile picture next to your story?"]) ``` to ``` processedFormDataObject.set("GitHub Picture Allowed ?", rawFormDataObject["Could we use your GitHub profile picture next to your story?"]) ```

Comparison

First correlation https://github.com/hackforla/website/blob/50af24636cc916192b5dc7d06d36ed8fe72767c0/google-apps-scripts/wins-form-responses/Code.js#L147 seems related to line 12 in the wins.js file https://github.com/hackforla/website/blob/a999c255a87d761e7579db1d76a6e0a0a3a05d39/assets/js/wins.js#L12
Second correlation https://github.com/hackforla/website/blob/50af24636cc916192b5dc7d06d36ed8fe72767c0/google-apps-scripts/wins-form-responses/Code.js#L148 seems related to line 13 in the wins.js file https://github.com/hackforla/website/blob/a999c255a87d761e7579db1d76a6e0a0a3a05d39/assets/js/wins.js#L13

Form headers (from the spreadsheet)

All columns that correspond to the questions - Email Address - Full name - Linkedin URL (optional) - Could we use your Linkedin profile picture next to your story? - Github URL (optional) - Could we use your Github profile picture next to your story? - Select the team(s) you're on - Select your role(s) on the team - What is/was your specific role? (optional) - When did you join Hack for LA? (optional) - What do you want to celebrate (select all that apply)? - Give us a brief overview
Additional form headers in addition to the form questions Timestamp Display? Homepage?
Form Question Changes Change form question from ``` Could we use your Github profile picture next to your story? ``` to ``` Could we use your GitHub profile picture next to your story? ```

Does the above seem to cover it all (excluding pages/wins/wins-share-form.html for now)? And if it does cover everything, what is your question.

daras-cu commented 1 month ago

@ExperimentsInHonesty Thank you for the clarification on issue size/role labels, that all makes sense.

Re: wins.js and Code.js I believe that does cover all the instances that need to be corrected in those files. My main question is for the lines with correlations to the Google form, how should I handle changing the questions on the form when updating the JS files so the question names remain in sync? (I do not have permission to view the spreadsheet you linked and assume other volunteers do not either)

If I create an issue for each form question that needs to be changed, the action items could be to edit the corresponding lines in both Code.js and wins.js, and update the question in the Google form. The assignee would have to be someone who has permissions for the form already, or would have to request permissions as part of working on the issue.

Alternatively, the issue could cover editing just Code.js and wins.js and then require a note to be left on the PR that the issue should only be merged by someone who can update the question on the Google form at the same time.

I'm also not sure about how testing would work for these issues. It appears that _data/external/_wins-data.json would have to be updated to reflect the new question names in order for wins.js to populate the Wins webpage correctly. It looks like _wins-data.json is updated by a bot account rewriting the file in its own branch then opening a PR (the code for this is in the main function in Code.js), which I'm not sure can be replicated in someone's forked repository. The issues I create could just call for the edits to the files without testing. Regardless, once the edited files are merged I think the _wins-data.json file will have to be updated immediately for the Wins webpage to display correctly, as otherwise the JSON data will still have the old question names as keys. Can we manually trigger an update after the changes are merged? If so how should I document that this needs to be done in the issues?

I hope all that is clear and that I'm understanding how the files work together correctly.

daras-cu commented 1 month ago

Update: I created issues #7494 and #7495 to fix two 'Github' instances in wins.js that are independent from the Wins form.

I have been familiarizing myself with the Google Apps Script development process and am working on getting access to the necessary Google docs to see how and where to make changes to the Wins form questions that include 'Github' and are referenced in Code.js and wins.js.

daras-cu commented 2 weeks ago

Update: The remaining issues to create for this ER involve making changes to the Wins form. As discussed with @roslynwythe, we should improve the documentation on Wins form development and testing before asking developers to work on these issues. I'll be going through the setup process for Wins issues locally and will note any areas where documentation can be improved. This will also help me better understand how to write the issues for this ER when we are ready for them.