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.

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

Dev user endpoint #25

Merged
merged 1 commit into from
Jul 15, 2024
Merged

Conversation

brngylni
Copy link
Contributor

@brngylni brngylni commented Jul 2, 2024

This PR adds the User endpoint #5

@gridhead gridhead requested review from abompard and gridhead and removed request for abompard July 3, 2024 07:06
@gridhead gridhead self-assigned this Jul 3, 2024
@gridhead gridhead requested a review from abompard July 3, 2024 07:07
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/user.py Show resolved Hide resolved
webhook_to_fedora_messaging/endpoints/user.py Outdated Show resolved Hide resolved
webhook_to_fedora_messaging/endpoints/user.py Outdated Show resolved Hide resolved
webhook_to_fedora_messaging/endpoints/user.py Outdated Show resolved Hide resolved
webhook_to_fedora_messaging/endpoints/user.py Show resolved Hide resolved
Comment on lines +2 to +3
from ..database import db
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.

Comment on lines 57 to 58


Copy link
Member

Choose a reason for hiding this comment

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

Leave one empty line at the end of the file.

webhook_to_fedora_messaging/endpoints/user.py Outdated Show resolved Hide resolved
@@ -15,7 +15,7 @@
from .config.defaults import LOGGER_CONFIG

from logging.config import dictConfig

from .endpoints.user import user_endpoint
Copy link
Member

Choose a reason for hiding this comment

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

@@ -26,7 +26,7 @@ def create_app():
main = Flask(
"webhook_to_fedora_messaging"
)

Copy link
Member

Choose a reason for hiding this comment

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

Leave the line empty.

Suggested change

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.

Please use the emojis to acknowledge the changes made and leave it for the reviewer to decide if the requested change has been made or not.

Once you are done with making the remaining changes - please squash the commits into one and then, it should be ready for a merge.

webhook_to_fedora_messaging/endpoints/user.py Outdated Show resolved Hide resolved
webhook_to_fedora_messaging/endpoints/util.py Outdated Show resolved Hide resolved
@brngylni brngylni requested a review from gridhead July 10, 2024 11:13
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.

LGTM

@brngylni, please resolve the merge conflict and squash all commits into one.

@brngylni
Copy link
Contributor Author

LGTM

@brngylni, please resolve the merge conflict and squash all commits into one.

Done!

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 38c6661 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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants