Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Dev service endpoint #24

Merged
merged 1 commit into from
Jul 15, 2024
Merged

Dev service endpoint #24

merged 1 commit into from
Jul 15, 2024

Conversation

brngylni
Copy link
Contributor

@brngylni brngylni commented Jul 2, 2024

This PR adds the service endpoint #7

@gridhead gridhead self-assigned this Jul 3, 2024
gridhead
gridhead requested changes Jul 8, 2024
View reviewed changes
Copy link
Member

@gridhead gridhead left a comment

Choose a reason for hiding this comment

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

Also, please follow #2 (comment).

webhook_to_fedora_messaging/endpoints/service.py
Comment on lines +2 to +4
from ..database import db
from ..models.service import Service
from ..models.user import User
Copy link
Member

Choose a reason for hiding this comment

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

@abompard Do we want to stick to relative or absolute imports for this one?

@brngylni Whatever it might be - Let us stick to just one for consistency.

webhook_to_fedora_messaging/endpoints/service.py Outdated Show resolved Hide resolved
webhook_to_fedora_messaging/endpoints/service.py Outdated Show resolved Hide resolved
webhook_to_fedora_messaging/endpoints/service.py Show resolved Hide resolved
webhook_to_fedora_messaging/endpoints/service.py Show resolved Hide resolved
webhook_to_fedora_messaging/endpoints/service.py Show resolved Hide resolved
webhook_to_fedora_messaging/endpoints/service.py Show resolved Hide resolved
webhook_to_fedora_messaging/endpoints/service.py Show resolved Hide resolved
webhook_to_fedora_messaging/endpoints/service.py Outdated Show resolved Hide resolved
webhook_to_fedora_messaging/endpoints/util.py Outdated Show resolved Hide resolved
This was referenced Jul 8, 2024
Merged
Closed
@brngylni brngylni requested a review from gridhead July 9, 2024 22:53
gridhead
gridhead requested changes Jul 10, 2024
View reviewed changes
Copy link
Member

@gridhead gridhead left a comment

Choose a reason for hiding this comment

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

@brngylni Please squash the commits and every other changes required on the codebase can be done after the merge.

@brngylni
Copy link
Contributor Author

@brngylni Please squash the commits and every other changes required on the codebase can be done after the merge.

Is it OK like this or do you want me to squash the remaining commits too?

@brngylni brngylni requested a review from gridhead July 11, 2024 13:58
@brngylni brngylni force-pushed the dev_service_endpoint branch 4 times, most recently from e57d888 to bcc8c1b Compare July 12, 2024 22:35
gridhead
gridhead approved these changes Jul 15, 2024
View reviewed changes
Copy link
Member

@gridhead gridhead left a comment

Choose a reason for hiding this comment

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

I am not dragging this any longer than it has to be. Other required changes can be made to the codebase after it is merged.

Merging now.

@gridhead gridhead merged commit de4b606 into fedora-infra:main Jul 15, 2024
3 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gridhead gridhead gridhead approved these changes

@abompard abompard Awaiting requested review from abompard

Assignees

@gridhead gridhead

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@brngylni @gridhead