-
Notifications
You must be signed in to change notification settings - Fork 208
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixes: #573. Add Basic Gitea support #1048
base: develop
Are you sure you want to change the base?
Conversation
Builds off PR GothenburgBitFactory#720 to add gitea integration to bugwarrior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for updating this to be compatible with the latest development branch!
As you noted, there are a number of things needed yet for us to be able to merge this. Here's what I'm noticing in a quick scan:
- documentation
- tests
- remove
six
Looking at some of the remnant TODO's from #720, I would note that additional features are not necessary before this is merged.
mish-mash between the github and gitlab documentation with everything I don't think is supported removed.
Suggestions from here are implemented GothenburgBitFactory#1048 (review)
I've never wrote python tests before, so I have not done it yet. Documentation is not very good, but I think gets the point across. The gitea feature here is rather indev, so it shouldn't be shocking. I would like to remove the username-password login combo since making tokens is easy in Gitea, and the username-password login combo tends to break when you use OTP (I enforce it on my instance). Do you think that would be a good idea? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another TODO:
- Add Gitea to the list of services at the top of README.rst
I would like to remove the username-password login combo since making tokens is easy in Gitea, and the username-password login combo tends to break when you use OTP (I enforce it on my instance). Do you think that would be a good idea?
I have no objection to removing password authentication. Many of the third-party services we integrate with have been deprecating and removing password authentication and I wouldn't be surprised if the rest follow suit.
Not sure if you'd seen this already or if you'll find it helpful but there is a brief section in the guide on adding tests: https://bugwarrior.readthedocs.io/en/latest/contributing/new-service.html#tests |
Remove user+pass auth, token only now. Added issue API Querying for writing custom queries Added include_assigned,created,mentioned, and review_requested issues config settings Added ability to limit the number of issues you will query (Gitea limits the API by default to 50, but I host my own instance so I raised it) get_tags simplified greatly
@@ -269,6 +273,9 @@ def to_taskwarrior(self): | |||
if body: | |||
body = body.replace('\r\n', '\n') | |||
|
|||
if len(body) < 1: | |||
body = "No annotation was provided." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about this. For consistency with the other services I think an empty body should be allowed.
self.host = self.config.host | ||
|
||
self.exclude_repos = self.config.exclude_repos | ||
|
||
self.include_repos = self.config.include_repos | ||
|
||
self.username = self.config.username | ||
self.filter_pull_requests = self.config.get( | ||
'filter_pull_requests', default=False, to_type=bool | ||
) | ||
self.exclude_pull_requests = self.config.get( | ||
'exclude_pull_requests', default=False, to_type=bool | ||
) | ||
self.involved_issues = self.config.get( | ||
'involved_issues', default=False, to_type=bool | ||
) | ||
self.import_labels_as_tags = self.config.get( | ||
'import_labels_as_tags', default=False, to_type=bool | ||
) | ||
self.label_template = self.config.get( | ||
'label_template', default='{{label}}', to_type=bool | ||
) | ||
self.project_owner_prefix = self.config.get( | ||
'project_owner_prefix', default=False, to_type=bool | ||
) | ||
|
||
self.filter_pull_requests = self.config.filter_pull_requests | ||
|
||
self.exclude_pull_requests = self.config.exclude_pull_requests | ||
|
||
self.involved_issues = self.config.involved_issues | ||
|
||
self.project_owner_prefix = self.config.project_owner_prefix | ||
|
||
self.include_assigned_issues = self.config.include_assigned_issues | ||
|
||
self.include_created_issues = self.config.include_created_issues | ||
|
||
self.include_review_requested_issues = self.config.include_review_requested_issues | ||
|
||
self.import_labels_as_tags = self.config.import_labels_as_tags | ||
|
||
self.label_template = self.config.label_template |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think all these instance variables should now be removed in favor of accessing self.config
directly.
include_assigned_issues: bool = False | ||
include_created_issues: bool = False | ||
include_mentioned_issues: bool = False | ||
include_review_requested_issues: bool = False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than adding a separate configuration option for these specific purposes, I would rather see a more powerful query
configuration option which can pass arbitrary filters to the gitea api. (This would apply to the issue_limit
option as well.) See github.query for example.
''' | ||
httpQuery = "limit=" + str(self.config.issue_limit) + "&" | ||
|
||
if self.config.get('include_assigned_issues', True, bool): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While I questioned whether these options are necessary above, I would note that the way you're accessing the configuration here and below doesn't make a lot of sense to me. You are defining the default values as False
in the GiteaConfig
class.
include_assigned_issues: bool = False
But here you change the effective default to be True
. It would make more sense to set the desired default in GiteaConfig
and access is simply with self.config.include_assigned_issues
.
This PR lets you use Gitea when using some basic configuration, built off #720.
To test this, you should use this as your working config, it's what's worked for me:
Fair warning, this is a hack-job that sucks and needs a lot of clean up, quality of life, documentation, and feature improvements. I'll help whenever convenient, but this patch gets the job done for me as-is.