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

Refactoring usermanager.py #1461

Draft
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

dominikatrojanowska
Copy link
Contributor

@dominikatrojanowska dominikatrojanowska commented Oct 21, 2024

Adding type hints, docstrings, and refactoring usermanage.py

@dominikatrojanowska dominikatrojanowska changed the title Draft: Previously selected proposal gets automatically selected on new login by another user Draft: Refactoring lims Oct 30, 2024
@dominikatrojanowska dominikatrojanowska changed the title Draft: Refactoring lims Draft: Refactoring usermanager.py Nov 4, 2024
@dominikatrojanowska dominikatrojanowska changed the title Draft: Refactoring usermanager.py Refactoring usermanager.py Nov 4, 2024
@marcus-oscarsson
Copy link
Member

Is this a draft pr, and what is it meant do do ?

@dominikatrojanowska
Copy link
Contributor Author

its initial purpose was a bit different that's where the branch name came from
so the changes here are only about simplifying logic in a few places and adding comments

@marcus-oscarsson
Copy link
Member

I see, Ill put this as draft for the moment

@marcus-oscarsson marcus-oscarsson marked this pull request as draft November 13, 2024 16:31
@dominikatrojanowska
Copy link
Contributor Author

as you wish, but I think I have no more changes for this PR, this is final version from my side
I had this idea that it's always good to have more documented code :)

Copy link
Contributor

@fabcor-maxiv fabcor-maxiv left a comment

Choose a reason for hiding this comment

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

Looks good to me. I think it can be merged.

@marcus-oscarsson I am not sure what led you to believe it might still be a draft. Probably because @dominikatrojanowska did not write a description on the pull request.

Looks to me like there are no logical changes, but it is adding type hints, docstrings, and refactoring.

@marcus-oscarsson
Copy link
Member

@dominikatrojanowska, how would you like to proceed. I can propose to try to carry over some of this work to a new PR (relatively soon) or you can try to resolve the conflicts. Let me know :)

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.

3 participants