Skip to content
This repository has been archived by the owner on May 16, 2022. It is now read-only.

commenting on Issues/PRs with ogr #229

Merged
merged 6 commits into from
Jul 8, 2019
Merged

Conversation

marusinm
Copy link
Collaborator

@marusinm marusinm commented Jul 2, 2019

Commenting on Issues and PRs are available for Github but also Pagure since now. I added two new variables into the private config file pagure_token and pagure_username. I'm just thinking if we should add this into README or wait till all Github features will be implemented also for Pagure.

Ogr doesn't have released commenting on issues yet. If you want to try locally you must install ogr from Github, not from requirements.txt
pip install git+https://github.com/packit-service/ogr@master

@jpopelka jpopelka changed the title commenting on Iussus/PRs with ogr commenting on Issues/PRs with ogr Jul 3, 2019
elif self.pagure_token != '' and self.pagure_username != '':
return self.get_pagure_project(username=self.pagure_username,
repository=self.repository_name,
token=self.pagure_token)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR should help you with that..;-) Feel free to add some comments. I would appreciate some opinions from the perspective of release-bot.

We can release a new version of OGR after merging that...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My current solution is easier for required options github_token and github_username in conf.yaml. Your PR creates ogr project instance from url. I can use it, but first, we need to discuss #230. Option clone_url in conf.yaml must be used.

Sorry, I missed adding a comment to your PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can discuss that in #230 ...

Copy link
Member

@jpopelka jpopelka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't tested but LGTM.
Does this fully implement #190 or are there any other parts which could be replaced with ogr?

@marusinm
Copy link
Collaborator Author

marusinm commented Jul 4, 2019

@jpopelka This definitely doesn't fully implement #190, I will go through release-bot functions and make smaller PRs to complete #190. This also needs to contribute to ogr.

I also deleted the test for adding a comment on Issue/PR since it is already tested in ogr. However, we will need to provided integration tests for Pagure and Github. But firstly, I assume to replace more API calls with ogr.

release_bot/configuration.py Outdated Show resolved Hide resolved
release_bot/configuration.py Outdated Show resolved Hide resolved
release_bot/configuration.py Outdated Show resolved Hide resolved
custom_instances=[
GithubService(token=self.github_token),
PagureService(token=self.pagure_token,
instance_url=self.pagure_instance_url)])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Contributor

@rpitonak rpitonak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! LGTM, merging...

@rpitonak rpitonak merged commit 753f5e2 into user-cont:master Jul 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants