Pulls languages from multiple repos#5037
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. |
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
@StephenTheDev1001 admire your persistence and hard work on this issue
you might consider adding an extra key/value pair to the civic-tech-index.md that contains the additional repo id and then just adding conditional logic to projects.js and current-projects.js that checks for if that property exists on the project object in order to display the extra languages on both the project details page and the project cards
that way you may be able to avoid having to add nested loops to both files
|
@MattPereira Should I make it to where we can add more than one additional repo to scrape from or is one additional repo enough? |
|
@StephenTheDev1001 I think one additional is fine since only this one project has multiple repos but I'm not up to date with website team affairs anymore @t-will-gillis @roslynwythe what do you think? |
@MattPereira I am not aware of any other project that would need more than one additional repo to scrape for language data. |
MattPereira
left a comment
There was a problem hiding this comment.
Looks good @StephenTheDev1001 !!!
Nice execution, solid communication with the team, and quick turnaround on the requested changes 👍
|
Hi @adrianang! If you plan to review this PR, please add your ETA and availability when you have a minute. If not, please remove yourself as reviewer. Thanks! |
|
adrianang
left a comment
There was a problem hiding this comment.
Hi @StephenTheDev1001 — the branching is set up correctly, the corresponding issue is linked, and the requested change has been made (ensure that Civic Tech Index shows all languages between its two repositories). In testing your branch on my local environment through Docker, I see the same thing that you had screenshotted for both the card view on the main Website page and the CTI project page view itself, and no other project seems to be affected as a result of these code changes.
I like the guard clauses that you set up, as a result of adding the second repo ID instead of appending it onto identification, as it minimizes the number of nested loop calls that the function makes (if I recall correctly, one of your previous commits called nested loops on each project). Nice job optimizing!
Thank you for sticking with the issue and seeing it through to completion; apologies that I couldn't be of more help, but it's great that you were able to find a pretty clever solution. Your efforts are deeply appreciated! 🙌🏼
* add repo id and turn project var into arr * merge arrays of languages and remove duplicates * add additional-repo-ids as separate * add moreProjectIds var * able to display languages from 2+ repos * add comment where data comes from * only working for one additional repo, not 3+ * project page showing languages from 2 repos * working changes to current-projects * remove some console.logs
Fixes #3805
What changes did you make?
-allow for 2 repo ids to be scraped
-merge lanaguages array from both repos without duplicates
-added CTI backend repo id to md file
Why did you make the changes (we will use this info to test)?
-the languages section now shows languages used in both CTI Frontend and CTI Backend
Screenshots of Proposed Changes Of The Website (if any, please do not screen shot code changes)
Visuals before changes are applied
Details
Visuals after changes are applied