Tracking tasks related to supporting other types of remote repo URLs in PR#1644

[chan-j-d]

The following are a list of necessary tasks related to but not implemented in PR#1644. This issue is meant to document and track the necessary changes.

• [Docs] Update UG, DG and codebase for direct references to GitHub to use more general language
• [Docs & Backend] Change author-config.json column header Author's GitHub ID to Author's Git Host ID while maintaining backward compatibility with older author-config.json files that still use Author's GitHub ID column header
  • Assigned to @SkyBlaise99 ([#1726] Update author config #1799)
• [Frontend] Add message or warning when unsupported links are clicked to notify user that the link is not supported
• [Frontend] Allow the repo icon to use respective github, gitlab, and bitbucket icons (fontawesome has all three) when we know the repo is one of them, but that can be a separate enhancement (taken from this suggestion)
  • Assigned to @SkyBlaise99 ([#1726] Use respective icons for known repository types #1800)
• [Tests] Expand list of repositories used for PR dashboard previews to include other git hosting platforms
  • Assigned to @SkyBlaise99 ([#1726] Add support for GitLab and Bitbucket test repos #1805)

[SkyBlaise99]

Hi, I wonder if this is still ongoing and if I can attempt it? I have an idea for the 2nd point regarding author-config.json.

[SkyBlaise99]

Hi @chan-j-d, any update on this?

[chan-j-d]

Hi @SkyBlaise99, apologies I missed your original message. You're welcome to attempt any of them! Do update me if you're attempting any of the others.

[SkyBlaise99]

@chan-j-d I have a question regarding my part about the backward compatibility.

Would you like the program to just be able to read from older author-config.json, or to also update the header from Author's GitHub ID to Author's Git Host ID also?

[SkyBlaise99]

The part on the backward compatibility might involve changing quite a lot of the codes. Currently the parsers recognize the header using the XXX_HEADER strings that we defined.

What I have in mind is to change this into an array that stores both the new and old headers. So something like

private static final String[] GIT_ID_HEADERS = {"Author's Git Host ID", "Author's GitHub ID"};

Doing so will allow the parser to scan through the array to ensure that none of the headers match. Good side to this will be that this can then be extended to any headers if we wish to change in the future. Down side will also be that I might need to change quite a bit of the code base.

What do you think?

[chan-j-d]

Would you like the program to just be able to read from older author-config.json, or to also update the header from Author's GitHub ID to Author's Git Host ID also?

I think I prefer the first solution, allowing the program to read the additional header. If I'm understanding what you mean by the second solution correctly, I think updating the user's file is not a simple task. What I had in mind is more in line with what you're suggesting in the next comment though I didn't actually think it would require a lot of changes.

[SkyBlaise99]

I think I prefer the first solution

Ok noted.

I didn't actually think it would require a lot of changes.

For this, I had already given it a quick try. Basically when I change to String[], a lot of the data/return types need to be changed as well.

For example, String[] mandatoryHeaders() will have to be changed to List<String[]> mandatoryHeaders(). And since it extends from CsvParser, the other files will also need to be updated as well. As a side effect, String[] optionalHeaders() is also affected.

Essentially, I'm expecting to change the return type of these 2 abstract methods to a List, and propagate those changes to places as required. That's why I say there might be quite a lot to change.

I'll try to get a PR up by mid of next week then you can see what I mean.

Update: I think I found a way to achieve fewer changes.

[SkyBlaise99]

I'm giving part 4 a try also.

[chan-j-d]

@SkyBlaise99 sure, you have been assigned it.

[SkyBlaise99]

Any chance there is some dummy repo from gitlab and bitbucket to be used directly?

[SkyBlaise99]

For part 3, does unsupported links refer to those links other than github, gitlab and bitbucket? If so, I can just do it together with part 4 to avoid future merge conflict.

Something like this for warning message? (just for example, I know GitHub is supported)

Also, since I'm testing if the icons are indeed working, part 5 is kind of covered as well.

Currently able to get all 3 icons out correctly. The default icon database is kept for whatever repo that isn't one of the 3, and currently there is no such repo.

[gok99]

There currently exists an issue #1689 documenting broken report links. You could see if it makes sense to fix those issues with this one, and possibly add details / discussion @SkyBlaise99.

[SkyBlaise99]

@gok99 do take a look here 😄

[gok99]

@SkyBlaise99 Yes, #1801 does look like a separate issue from #1689. Task 3 in this issue on unsupported links seems fairly broad, so a fix would likely need to address both #1801 (probably unsupported unless the profile pages exist) and #1689 (definitely unsupported currently) in some fashion.

[SkyBlaise99]

A bit confusing to me. I need to clarify this first.

I raised #1801 because the supported repos' links are not working. Unless I didn't understand what 'supported' here refers to.

I believe that Github, Gitlab, and Bitbucket are the only 3 supported currently, is that corrected?

On that basis, the current situation is as such:

Github: authors link working
Gitlab: shows 404
Bitbucket: shows 404

For Gitlab, there is a link to authors profile, but not the url that is currently used, hence leading it to 404. So there is a need to update the getRepoLink method in the frontend side.

For Bitbucket, I cannot find the profile page. So perhaps we can hide it until someone manage to find the correct link. Then follow as above.

The fix for #1801 should be oriented on getting the correct link, and not hiding the button like mentioned in #1689. So a fix for #1689 and maybe task 3 shouldn't be able to fix #1801.

Essentially, to fix #1689, your proposed way is to remove or hide the button. To fix #1801, my proposed way is to fix the url.

[gok99]

@SkyBlaise99 oh yes, apologies for the confusion (I should have specified what I was responding to).

I meant to suggest that the two issues are in fact separate with separate fixes.

I was responding to your earlier reply to this issue on part 3's warning message where issue #1689 would be relevant.

Regarding the meaning of 'unsupported' I think we can discuss if it's worth the trouble explicitly handling the Bitbucket profile links the same way we do the non GitHub/GitLab/Bitbucket links

[SkyBlaise99]

@gok99 OK now I get what you mean. For task 3, I agree it is similar to #1689. In fact the only difference should be the handling of merge conflict actually. Perhaps @chan-j-d can take a look and decide what to do with task 3.

As for #1801, I agree that more discussion would be needed before implementing the solve. We can discuss more there.

[chan-j-d]

Apologies for the confusion. My original intention with part 3 was a direct result of the way I implemented handling of remote repo link generation that isn't GitHub, GitLab or BitBucket in that the user is redirected to a URL that is some variant of UNSUPPORTED which was not a good look. But it seems the core issue directly overlaps with #1689.

Also for this,

For part 3, does unsupported links refer to those links other than github, gitlab and bitbucket? If so, I can just do it together with part 4 to avoid future merge conflict.

Probably shouldn't combine PRs together just to avoid merge conflicts. Separating allows for easier independent reviews and more atomic commits. The two parts are quite different in their functions.

[SkyBlaise99]

Ok, but my part 4 indirectly covers part 5 as a result of me adding the new repos to try out if the icons are working. But I think this is fine?

[chan-j-d]

I don't think part 5 is quite that simple. I'll mention first that I was not the one who thought of part 5 for this issue. My understanding of it would be to not just add gitlab and bitbucket repos to repo-config.csv but also create an 'official' reposense organization and relevant test repos to be featured in the dashboard preview.

Perhaps @dcshzj can help clarify

[SkyBlaise99]

@dcshzj any updates on part 5?

[dcshzj]

Hello @SkyBlaise99, my apologies. I can't remember exactly the context when I added part 5, but I believe my intent was to add more repositories that clones from other git hosting providers. It does not necessarily need to be test repos, but just a generic repository so that we can verify that RepoSense works for places other than GitHub. With #1800, it also helps to ensure that code designed for other git hosting providers are not broken.

I think it would be sufficient for now to pull from repositories not connected to RepoSense, and are small enough that it does not cause our builds to take too long. If there is sufficient need in the future, we can look at creating the RepoSense organizations in the other git hosting platforms.

[SkyBlaise99]

So I take it that part 5 is purely adding 2 repos that are not GitHub and testing out if the current code is able to work with them? Then how about my proposed solution for part 5 above? Just add 2 dummy repos I created? Would there be needs for more test cases?

[dcshzj]

Yup I think using 2 dummy repos is sufficient, I doubt there would be more test cases needed.

[SkyBlaise99]

Ok. In that case, can you assign me part 5 and I'll add my test repos back @chan-j-d? Also, can you try accessing the GitLab repo again to double-check if it is now made public?

[chan-j-d]

It has been assigned to you. I would request that you also add some commits to the dummy repos such that when it shows up in the dashboard, there are also commits to click on in order to test those links

[SkyBlaise99]

I think I already have 1 commit for both repos (updating of readme.md). Is that enough or would you want more?

[chan-j-d]

Maybe get it up to about 5. I don't think this might happen but having multiple different commits to test on would give me more confidence

[SkyBlaise99]

Sure. I will make something up haha

[ckcherry23]

@chan-j-d Do you think the one remaining task would be suitable for first-timers?

[chan-j-d]

This seems like a pretty mundane and tedious task for first timers. I think not as it is not immediately obvious what is expected and how to go about doing it. I should really go about to finishing this soon.