Refactor GitLab integration #1116
Conversation
Ndpnt
force-pushed
the
add-gitlab-functionalities-update
branch
from
e99a4de to
e0c4cbf
Compare
Add code to handle Gitlab repository, add axios to project packages, add env variables for Gitlab
Add test file for GitLab, fix formatting in files, refactor GitLab code to remove axios library
Remove the GitLab releases repository with a new one used as an example
Remove duplicate readme for gitlab and handle the new input parameter
Fix the release zip workflow using GitLab packages to upload the file
Aligned format and minor wording fixes
Co-authored-by: Matti Schneider <[email protected]>
Ndpnt
force-pushed
the
add-gitlab-functionalities-update
branch
from
e0c4cbf to
aed17f3
Compare
|
My mistake, I saw that this was moved into the json configuration file, with "baseURL" and "apiBaseURL". Please ignore this comment, thank you. |
|
Related documentation update OpenTermsArchive/docs#147 |
|
@alessandrozago Could you please confirm that it works as expected with GitLab? |
Hi @Ndpnt , we did some preliminary testing on the version of commit aed17f3, and the only issue we found at the moment is that the code still references the old parameter "versionsRepositoryURLGitLab" instead of the generic "versionsRepositoryURL".
When fixed we are available to test it again with also the latest changes from today's commits. Thank you |
Thank you for your feedback! I’ve addressed the issue regarding the old parameter "versionsRepositoryURLGitLab" in the code. The references have been updated to use the generic "versionsRepositoryURL" as per the latest documentation. Feel free to test again with today's latest commits. Let me know if anything else comes up! |
src/reporter/index.test.js
Outdated
|
|
||
| expect(() => { | ||
| Reporter.validateConfiguration(repositories); | ||
| }).to.throw('Required configuration key "reporter.repositories.declarations" was not found; issues on the declarations repository cannot be created'); |
There was a problem hiding this comment.
I recommend checking against just a significant subset of the string rather than the entire error message, to maximise tests maintainability.
src/reporter/index.js
Outdated
| } | ||
|
|
||
| /** | ||
| * Support for legacy config format where reporter configuration was nested under "githubIssues" |
There was a problem hiding this comment.
| * Support for legacy config format where reporter configuration was nested under "githubIssues" | |
| * Support for legacy config format where reporter configuration was nested under `githubIssues` |
|
@alessandrozago Could you also review the documentation update PR to ensure it’s clear and reflects the recent GitLab updates? |
Hi @Ndpnt , thanks for the fix on the parameter. Another small thing is that I left a logger line on debug level with no real indication for the final user ( /src/reporter/gitlab/index.js:265 ). I can remove it on my fork (or change the message to something more clear) but this pull request does not seem to be linked, so it should not update the code. Please let me know if I should do some action to help on this. We did not find any other issues that can impact the functionalities. So, aside from the error on the cache mentioned above, on our side it is ok to proceed. Please let us know if you would like any other action from our side. |
|
Thanks @alessandrozago for the valuable tests! |
Co-authored-by: Matti Schneider <[email protected]>
Thanks @alessandrozago to the work done in #1098 to support GitLab as an alternative to GitHub for both issue reporting and dataset publishing. 🎉
This PR refactors the initial implementation to reduce the code footprint needed to support GitLab and improve overall consistency across the codebase.
@MattiSG, could you please review these changes first? Once reviewed, we’ll ask @alessandrozago to perform additional testing with GitLab to confirm everything works as expected and offer any feedback.