Skip to content

Added/improved docstrings in Factory code #511

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

Merged
merged 2 commits into from
Apr 24, 2025

Conversation

mambelli
Copy link
Contributor

@mambelli mambelli commented Apr 7, 2025

Added/improved docstrings in Factory code

Also fixed some Python3 compliance in a couple of factory tools.

For the docstrings, I used in part ChatGPT:

  • definitely needs supervision
  • does not handle big files well
  • skips sections or repeats sections

Also made some PEP8 improvement, e.g. renamed glideFactoryDowntimeLib methods
@mambelli mambelli force-pushed the v310/factory_docstrings branch from 9001ba8 to 5cd011f Compare April 7, 2025 16:27
@mambelli mambelli requested a review from namrathaurs April 9, 2025 15:12
@mambelli mambelli self-assigned this Apr 9, 2025
Copy link
Contributor

@namrathaurs namrathaurs left a comment

Choose a reason for hiding this comment

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

Review feedback part 1/2: this round of review covers a major chunk of files that were modified (excluding files with large diffs which will be covered in part 2/2).

@mambelli mambelli force-pushed the v310/factory_docstrings branch from 5cd011f to 12169b9 Compare April 18, 2025 14:57
Copy link
Contributor

@namrathaurs namrathaurs left a comment

Choose a reason for hiding this comment

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

Review feedback part 2/2: this round of review covers files with large diffs

@mambelli mambelli force-pushed the v310/factory_docstrings branch 4 times, most recently from 2e8bf81 to f8c410d Compare April 24, 2025 14:58
@mambelli mambelli force-pushed the v310/factory_docstrings branch from f8c410d to 4d15a94 Compare April 24, 2025 16:56
Copy link
Contributor

@namrathaurs namrathaurs left a comment

Choose a reason for hiding this comment

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

All review comments have been addressed/resolved and these changes are ready to be merged!

@mambelli mambelli merged commit dec3739 into glideinWMS:master Apr 24, 2025
9 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