Closed spier closed 3 years ago
@mehrdadrad Take a look at this when you get a chance, to check if this is implementing this feature as you imagined it to work.
You should be able to try it out based on this branch. When collecting the data for your repos.json
, make sure to include the topics
property for every repo, potentially using the application/vnd.github.mercy-preview+json
media type when making GitHub API requests (see docs here).
Excellent, thanks for quick action! LGTM
@Michadelic my implementation is currently just the happy path, without any consideration for edge cases. Also lacks documentation.
Would you mind reviewing this and letting me know what I should add to get this PR mergeable?
@Michadelic something that I discovered while developing on this:
I accidentally committed (and then reverted) a different local version of repos.json
. I tried to add repos.json to .gitignore
but that didn't work, maybe because the file is already under version control? A bit cumbersome, at least when testing with different datasets locally.
@Michadelic something that I discovered while developing on this: I accidentally committed (and then reverted) a different local version of
repos.json
. I tried to add repos.json to.gitignore
but that didn't work, maybe because the file is already under version control? A bit cumbersome, at least when testing with different datasets locally.
I see an entry for repos.json
in .gitignore
:
https://github.com/SAP/project-portal-for-innersource/blob/main/.gitignore#L2
Is it in the wrong notation. At least in my IDE it works as designed
I just realized that as the most minimal form of documentation we should at least modify repos.json
to contain some repo objects that contain the topics
property. Without that it won't work when people check out this repo and just want to run it as-is first.
Will do that in a bit.
As a quick way to generate topics for each repo, I ended up copying properties that the repo already has into the topics
property.
How it did it:
cat repos.json | jq '[.[] | . + { "topics" : [.name, .language, .owner.login] }]'
This results in e.g. "Earth" having these topics now:
"topics": [
"earth",
"JavaScript",
"Sol"
]
I also modified the planets/repos Mars, Ceres, Venus manually, in order to test the edge cases of
If you run this branch locally now, you can see it in full effect.
Example:
@Michadelic if you are happy with how this works now, feel free to merge.
Yes, that case is covered as well.
I used the first 4 planets/repos that are shown at the very top when launching the app.
All repos have exactly 3 topics (with exception of the ones listed above). Mars has that many repos to test how the line-breaks and formatting works with that many topics, so I would suggest to leave it that way.
Implements #11.
This assumes that
repos.json
will contain a property like this for every repo:Changes:
Todo:
topics
property needs to be available for every repo