canonical / discourse-gatekeeper

Experimental GitHub Action to upload charm documentation to charmhub
Apache License 2.0
7 stars 7 forks source link

[MISC] Rebranding into Discourse Gatekeeper #210

Closed deusebio closed 12 months ago

deusebio commented 1 year ago

Overview

We should rebrand the project as "Discourse Gatekeeper" following the decision about the naming for this project.

:warning: IMPORTANT :warning: After merging this PR, we should probably also change the name of the repository into discourse-gatekeeper

Rationale

Daniele is suggesting us to rebrand, as the name upload-charm-docs is really not quite reflecting the true aim of this tool

Checklist

deusebio commented 1 year ago

Uhm, I'm not sure how breaking these changes are and how the deprecation should play out (if done within the same repo, see below). I mean, these constants are not provided by the user, but are used internally, and exposed in the form of tag names, branch names, pull-requests infos. Note that if the tag with the name does not exists, it gets created anyway, fixing the repo itself. Given this, I'm afraid that creating extra code to handle both naming in the same repository would only increase the engineering burder

The biggest change is actually probably changing the name of the repository (since it then requires people to update their actions), but there is no deprecation we can do about this.

One way to go could be to fork this project into a new repository with the right name (discourse-gatekeeper) and merge this PR there. In this repository we place a logging saying that this action is now deprecated (and not supported), nudging people to use the other one that will be the supported one. This should also allow us to have cleaner code

jdkandersson commented 1 year ago

Uhm, I'm not sure how breaking these changes are and how the deprecation should play out (if done within the same repo, see below). I mean, these constants are not provided by the user, but are used internally, and exposed in the form of tag names, branch names, pull-requests infos. Note that if the tag with the name does not exists, it gets created anyway, fixing the repo itself. Given this, I'm afraid that creating extra code to handle both naming in the same repository would only increase the engineering burder

The biggest change is actually probably changing the name of the repository (since it then requires people to update their actions), but there is no deprecation we can do about this.

One way to go could be to fork this project into a new repository with the right name (discourse-gatekeeper) and merge this PR there. In this repository we place a logging saying that this action is now deprecated (and not supported), nudging people to use the other one that will be the supported one. This should also allow us to have cleaner code

You are right actually, if the tag gets re-created that is fine. I think that GitHub maintains redirects so the renaming shouldn't be an issue

deusebio commented 1 year ago

@jdkandersson should we merge this PR? I have merged latest update of main, but I believe one reviewer is still required to have a look and approve the PR

jdkandersson commented 1 year ago

@jdkandersson should we merge this PR? I have merged latest update of main, but I believe one reviewer is still required to have a look and approve the PR

I'll ask for a review from the team

deusebio commented 1 year ago

I had to fix some conflicts on src-docs, which dismissed reviews.

Please reapprove @jdkandersson and @yanksyoon (BTW, I hope I also replied to your minors)

github-actions[bot] commented 1 year ago

Test coverage for 170d183a0e3216bbfc1da1d961475dfc25d96bed

Name                      Stmts   Miss Branch BrPart  Cover   Missing
---------------------------------------------------------------------
src/__init__.py              83      0     34      0   100%
src/action.py               155      0     46      0   100%
src/check.py                 53      0     21      0   100%
src/clients.py               12      0      0      0   100%
src/commit.py                42      0     12      0   100%
src/constants.py             10      0      0      0   100%
src/content.py               50      0     10      0   100%
src/discourse.py            159      0     34      0   100%
src/docs_directory.py        33      0      8      0   100%
src/download.py              23      0      2      0   100%
src/exceptions.py            15      0      0      0   100%
src/index.py                142      0     56      0   100%
src/metadata.py              28      0     12      0   100%
src/migration.py             87      0     27      0   100%
src/navigation_table.py      65      0     20      0   100%
src/reconcile.py            123      0     60      0   100%
src/repository.py           284      0     88      0   100%
src/sort.py                  43      0     26      0   100%
src/types_.py               196      0     54      0   100%
---------------------------------------------------------------------
TOTAL                      1603      0    510      0   100%

Static code analysis report

Working... ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 100% 0:00:02
Run started:2023-09-26 21:18:30.589818

Test results:
    No issues identified.

Code scanned:
    Total lines of code: 17033
    Total lines skipped (#nosec): 3
    Total potential issues skipped due to specifically being disabled (e.g., #nosec BXXX): 0

Run metrics:
    Total issues (by severity):
        Undefined: 0
        Low: 0
        Medium: 0
        High: 0
    Total issues (by confidence):
        Undefined: 0
        Low: 0
        Medium: 0
        High: 0
Files skipped (0):
deusebio commented 1 year ago

@yanksyoon could you re-approve? I had to rebase the branch, but that cleared reviews :(

jdkandersson commented 12 months ago

@yanksyoon could you re-approve? I had to rebase the branch, but that cleared reviews :(

He's been away unwell, I'll ask for review in our team

deusebio commented 12 months ago

He's been away unwell, I'll ask for review in our team

Oh, I see :( . I hope he will recover and get better soon! @yanksyoon