Add support for Forgejo.

Review Request #14607 — Created Sept. 10, 2025 and submitted

Information

Review Board
release-7.1.x

Reviewers

Forgejo is a newish open-source code forge system forked from Gitea
which has been rapidly gaining popularity. This change adds hosting
service support for it.

This implementation handles all of the main parts of integration for the
hosting service, including:

  • Authorization with the service, creating a new API token for the user
    with the correct permissions.
  • Basic support for getting file content (git blobs).
  • Post-commit review support, fetching branches, lists of commits, commit
    data, and diffs.
  • Bug tracker integration, including linking to issues and showing the
    bug infobox.
  • WebHook support for closing review requests when commits are pushed.

The only hosting service feature which is not implemented is the remote
repository listing.

  • Tested authorization and API token creation, including when 2FA was
    enabled.
  • Verified pre-commit and post-commit review functionality against a
    self-hosted Forgejo server.
  • Checked linking to bugs and the bug infobox.
  • Verified that setting up a WebHook according to the instructions worked
    correctly, and that pushing code to Forgejo properly closed relevant
    review requests.
  • Ran unit tests.
Summary ID
Add support for Forgejo.
Forgejo is a newish open-source code forge system forked from Gitea which has been rapidly gaining popularity. This change adds hosting service support for it. This implementation handles all of the main parts of integration for the hosting service, including: - Authorization with the service, creating a new API token for the user with the correct permissions. - Basic support for getting file content (git blobs). - Post-commit review support, fetching branches, lists of commits, commit data, and diffs. - Bug tracker integration, including linking to issues and showing the bug infobox. - WebHook support for closing review requests when commits are pushed. The only hosting service feature which is not implemented is the remote repository listing. Testing Done: - Tested authorization and API token creation, including when 2FA was enabled. - Verified pre-commit and post-commit review functionality against a self-hosted Forgejo server. - Checked linking to bugs and the bug infobox. - Verified that setting up a WebHook according to the instructions worked correctly, and that pushing code to Forgejo properly closed relevant review requests. - Ran unit tests.
zupounommokpsrpqoprqqltlnnwoyztv
Description From Last Updated

continuation line unaligned for hanging indent Column: 25 Error code: E131

reviewbotreviewbot

continuation line unaligned for hanging indent Column: 13 Error code: E131

reviewbotreviewbot

continuation line unaligned for hanging indent Column: 13 Error code: E131

reviewbotreviewbot

continuation line unaligned for hanging indent Column: 13 Error code: E131

reviewbotreviewbot

continuation line unaligned for hanging indent Column: 13 Error code: E131

reviewbotreviewbot

closing bracket does not match indentation of opening bracket's line Column: 13 Error code: E123

reviewbotreviewbot

closing bracket does not match indentation of opening bracket's line Column: 9 Error code: E123

reviewbotreviewbot

continuation line unaligned for hanging indent Column: 13 Error code: E131

reviewbotreviewbot

continuation line unaligned for hanging indent Column: 13 Error code: E131

reviewbotreviewbot

continuation line unaligned for hanging indent Column: 13 Error code: E131

reviewbotreviewbot

continuation line unaligned for hanging indent Column: 13 Error code: E131

reviewbotreviewbot

closing bracket does not match indentation of opening bracket's line Column: 13 Error code: E123

reviewbotreviewbot

closing bracket does not match indentation of opening bracket's line Column: 9 Error code: E123

reviewbotreviewbot

continuation line under-indented for visual indent Column: 25 Error code: E128

reviewbotreviewbot

continuation line under-indented for visual indent Column: 25 Error code: E128

reviewbotreviewbot

continuation line unaligned for hanging indent Column: 20 Error code: E131

reviewbotreviewbot

line too long (80 > 79 characters) Column: 80 Error code: E501

reviewbotreviewbot

continuation line under-indented for visual indent Column: 13 Error code: E128

reviewbotreviewbot

continuation line under-indented for visual indent Column: 13 Error code: E128

reviewbotreviewbot

continuation line under-indented for visual indent Column: 25 Error code: E128

reviewbotreviewbot

line too long (80 > 79 characters) Column: 80 Error code: E501

reviewbotreviewbot

Typo: Initialie

maubinmaubin

Get rid of the "is" here.

maubinmaubin

Would should handle the potential ValidationError here as well.

maubinmaubin

We have error = data.decode() above so we can pass that to .format() instead of decoding again. Might as well …

maubinmaubin

Since we can only hit this case if a two factor auth code was already provided by the user, I …

maubinmaubin

Was this meant to say "username" instead of "login"?

maubinmaubin

Since this error is moreso user facing than just admin facing (it'd be displayed in the diff viewer, right?) maybe …

maubinmaubin

We can get rid of this text.

maubinmaubin

Can you type these as well.

maubinmaubin

What if both sha and branch are not provided, shouldn't we use the default branch? Should have a unit test …

maubinmaubin

Can you add this to the "Raises" docs as well.

maubinmaubin

Note that this is still TODO.

maubinmaubin

Is this correct syntax? Does it work for RB7.1's supported Python versions?

maubinmaubin

This can fit on one line. Or are we always meant to put the format args on the next line …

maubinmaubin

Nit: I think it'd be nice to standardize on including a trailing slash in the URLs, for methods that return …

maubinmaubin

This could also raise HostingServiceAPIError (update the docstring in the corresponding ForgejoClient method too).

maubinmaubin

This could also raise HostingServiceAPIError (update the docstring in the corresponding ForgejoClient method too).

maubinmaubin

This could also raise HostingServiceAPIError (update the docstring in the corresponding ForgejoClient method too).

maubinmaubin

Do you know if Forgejo has a limit on API token name length? If so we'll have to cap it …

maubinmaubin

These should be swapped.

chipx86chipx86

I think both of these can live in TYPE_CHECKING.

chipx86chipx86

Can we pull this out into a separate variable? Gets a bit wordy with a call inside this, and one …

chipx86chipx86

Are we able to assert this instead of casting?

chipx86chipx86

We can probably remove the prefix, since the logger will have those details.

chipx86chipx86

We should escape the username.

chipx86chipx86

Lowercase "application".

chipx86chipx86

We may want to set up and reference a trace ID in this, point people to the logs.

chipx86chipx86

Can we stick with one per line, like a dictionary?

chipx86chipx86

We should escape the SHA.

chipx86chipx86

Same question here and probably below re: asserting.

chipx86chipx86

Same as above, let's stick with one keyword argument per line. Makes it easier to read and maintain.

chipx86chipx86

We should escape the revision.

chipx86chipx86

Same here.

chipx86chipx86

This is accessed 2 times per line. Let's pull it out into a local variable in the get_change.

chipx86chipx86

Is there consistency in how long the partial SHA is? If so, we can clip these in all_shas above and …

chipx86chipx86

Same here.

chipx86chipx86

We should escape bug_id.

chipx86chipx86

We should escape these.

chipx86chipx86

We should escape the commit.

chipx86chipx86

The "subclasses" part should be reworked.

chipx86chipx86

We should escape the repository owner and name.

chipx86chipx86

To make this a tad less verbose, can we pull out service_class?

chipx86chipx86
Checks run (1 failed, 1 succeeded)
flake8 failed.
JSHint passed.

flake8

david
Review request changed
Commits:
Summary ID
Add support for Forgejo.
Forgejo is a newish open-source code forge system forked from Gitea which has been rapidly gaining popularity. This change adds hosting service support for it. This implementation handles all of the main parts of integration for the hosting service, including: - Authorization with the service, creating a new API token for the user with the correct permissions. - Basic support for getting file content (git blobs). - Post-commit review support, fetching branches, lists of commits, commit data, and diffs. - Bug tracker integration, including linking to issues and showing the bug infobox. - WebHook support for closing review requests when commits are pushed. The only hosting service feature which is not implemented is the remote repository listing. Testing Done: - Tested authorization and API token creation, including when 2FA was enabled. - Verified pre-commit and post-commit review functionality against a self-hosted Forgejo server. - Checked linking to bugs and the bug infobox. - Verified that setting up a WebHook according to the instructions worked correctly, and that pushing code to Forgejo properly closed relevant review requests. - Ran unit tests.
zupounommokpsrpqoprqqltlnnwoyztv
Add support for Forgejo.
Forgejo is a newish open-source code forge system forked from Gitea which has been rapidly gaining popularity. This change adds hosting service support for it. This implementation handles all of the main parts of integration for the hosting service, including: - Authorization with the service, creating a new API token for the user with the correct permissions. - Basic support for getting file content (git blobs). - Post-commit review support, fetching branches, lists of commits, commit data, and diffs. - Bug tracker integration, including linking to issues and showing the bug infobox. - WebHook support for closing review requests when commits are pushed. The only hosting service feature which is not implemented is the remote repository listing. Testing Done: - Tested authorization and API token creation, including when 2FA was enabled. - Verified pre-commit and post-commit review functionality against a self-hosted Forgejo server. - Checked linking to bugs and the bug infobox. - Verified that setting up a WebHook according to the instructions worked correctly, and that pushing code to Forgejo properly closed relevant review requests. - Ran unit tests.
zupounommokpsrpqoprqqltlnnwoyztv

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

david
maubin
  1. 
      
  2. Show all issues

    Typo: Initialie

  3. Show all issues

    Get rid of the "is" here.

  4. Show all issues

    Would should handle the potential ValidationError here as well.

  5. Show all issues

    We have error = data.decode() above so we can pass that to .format() instead of decoding again. Might as well make a variable for e.code too and use that in the error log and here.

  6. reviewboard/hostingsvcs/forgejo/client.py (Diff revision 3)
     
     
     
    Show all issues

    Since we can only hit this case if a two factor auth code was already provided by the user, I feel like a more appropriate message would just be "Invalid two-factor authentication code."

    1. We get this both if the code was invalid but also when we first try to authenticate with just username+password (which is what makes us show the OTP field in the first place)

  7. Show all issues

    Was this meant to say "username" instead of "login"?

  8. reviewboard/hostingsvcs/forgejo/client.py (Diff revision 3)
     
     
     
     
    Show all issues

    Since this error is moreso user facing than just admin facing (it'd be displayed in the diff viewer, right?) maybe we should say something like "Please contact your administrator to review the Review Board log files ...".

    1. I'll just simplify it to the first sentence. I feel like contacting an admin is a predictable step if someone is getting a weird error.

  9. reviewboard/hostingsvcs/forgejo/client.py (Diff revision 3)
     
     
     
     
    Show all issues

    We can get rid of this text.

  10. reviewboard/hostingsvcs/forgejo/client.py (Diff revision 3)
     
     
     
     
    Show all issues

    Can you type these as well.

  11. reviewboard/hostingsvcs/forgejo/client.py (Diff revision 3)
     
     
     
    Show all issues

    What if both sha and branch are not provided, shouldn't we use the default branch? Should have a unit test for this case as well.

    1. The calling code will always provide one. I'll add some validation for this case.

  12. Show all issues

    Note that this is still TODO.

  13. Show all issues

    Is this correct syntax? Does it work for RB7.1's supported Python versions?

    1. Ah nope, was still syntax from when this was on the master branch.

  14. reviewboard/hostingsvcs/forgejo/client.py (Diff revision 3)
     
     
     
    Show all issues

    This can fit on one line. Or are we always meant to put the format args on the next line irregardless of the length of the string? I forget.

  15. Show all issues

    Nit: I think it'd be nice to standardize on including a trailing slash in the URLs, for methods that return URLs. So that in the future if someone's using _get_api_root or _get_api_repo_root, they wouldn't have to check which one has a trailing slash and which one doesn't.

    1. Forgejo API URLs don't use trailing slashes. I think I'll change it so _get_api_root() returns it without, and then we always add a slash where we use it.

  16. reviewboard/hostingsvcs/forgejo/service.py (Diff revision 3)
     
     
     
     
    Show all issues

    This could also raise HostingServiceAPIError (update the docstring in the corresponding ForgejoClient method too).

    1. HostingServiceAPIError is a subclass of HostingServiceError, and the code that calls these things pretty universally is only catching HostingServiceError

  17. reviewboard/hostingsvcs/forgejo/service.py (Diff revision 3)
     
     
     
     
    Show all issues

    This could also raise HostingServiceAPIError (update the docstring in the corresponding ForgejoClient method too).

  18. reviewboard/hostingsvcs/forgejo/service.py (Diff revision 3)
     
     
     
     
    Show all issues

    This could also raise HostingServiceAPIError (update the docstring in the corresponding ForgejoClient method too).

  19. Show all issues

    Do you know if Forgejo has a limit on API token name length? If so we'll have to cap it here, in case someone's server name is super long.

    1. Looks like in their UI the field is limited to 255.

  20. 
      
david
maubin
  1. 
      
  2. Also just making a note here that we'll need to update the Review Board documentation for Forgejo.

  3. reviewboard/hostingsvcs/forgejo/client.py (Diff revisions 3 - 4)
     
     
    Show all issues

    Can you add this to the "Raises" docs as well.

  4. 
      
david
chipx86
  1. 
      
  2. reviewboard/hostingsvcs/forgejo/client.py (Diff revision 5)
     
     
     
     
    Show all issues

    These should be swapped.

  3. reviewboard/hostingsvcs/forgejo/client.py (Diff revision 5)
     
     
     
    Show all issues

    I think both of these can live in TYPE_CHECKING.

  4. Show all issues

    Can we pull this out into a separate variable? Gets a bit wordy with a call inside this, and one of my plans around 8/9 timeframe's going to be revisiting calls like decrypt_password() to enable rotatable keys, which will require some additional complexity.

  5. Show all issues

    Are we able to assert this instead of casting?

  6. Show all issues

    We can probably remove the prefix, since the logger will have those details.

  7. Show all issues

    We should escape the username.

  8. Show all issues

    Lowercase "application".

  9. Show all issues

    We may want to set up and reference a trace ID in this, point people to the logs.

  10. Show all issues

    Can we stick with one per line, like a dictionary?

  11. Show all issues

    We should escape the SHA.

  12. Show all issues

    Same question here and probably below re: asserting.

  13. Show all issues

    Same as above, let's stick with one keyword argument per line. Makes it easier to read and maintain.

  14. Show all issues

    We should escape the revision.

  15. Show all issues

    Same here.

  16. Show all issues

    This is accessed 2 times per line. Let's pull it out into a local variable in the get_change.

  17. reviewboard/hostingsvcs/forgejo/client.py (Diff revision 5)
     
     
     
     
    Show all issues

    Is there consistency in how long the partial SHA is? If so, we can clip these in all_shas above and then just do a lookup.

    1. Not necessarily. It's at least 6, but in repos with lots of commits (like RB's) where that could create conflicts, partial SHAs are longer.

  18. Show all issues

    Same here.

  19. Show all issues

    We should escape bug_id.

  20. Show all issues

    We should escape these.

  21. Show all issues

    We should escape the commit.

  22. Show all issues

    The "subclasses" part should be reworked.

  23. Show all issues

    We should escape the repository owner and name.

  24. reviewboard/hostingsvcs/tests/test_forgejo.py (Diff revision 5)
     
     
     
     
     
     
     
     
    Show all issues

    To make this a tad less verbose, can we pull out service_class?

  25. 
      
david
maubin
  1. Ship It!
  2. 
      
chipx86
  1. Ship It!
  2. 
      
david
Review request changed
Status:
Completed
Change Summary:
Pushed to release-7.1.x (7d60ff0)