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

Feature/8 refactoring oop data containers #16

Merged
merged 34 commits into from
Jul 17, 2024

Conversation

MobiTikula
Copy link
Collaborator

@MobiTikula MobiTikula commented May 28, 2024

Script refactoring for better readability and efficiency using OOP.
Used dataclasses containers, for better working with well known structures.

Solves #8

…ng README file for correct executing scripts locally.
…h better documentation, structure, having more atomic functions, importing OOP with dataclasses etc.
@MobiTikula MobiTikula added the refactoring Improving code quality, paying off tech debt, aligning APIs label May 28, 2024
@MobiTikula MobiTikula self-assigned this May 28, 2024
Copy link
Collaborator

@miroslavpojer miroslavpojer left a comment

Choose a reason for hiding this comment

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

  • pulled
  • code review
  • run - failed

`
Error running consolidate_feature_data.py:
Environment variables:
PROJECT_STATE_MINING: True
REPOSITORIES: [{'orgName': 'absa-group', 'repoName': 'living-doc-example-project', 'queryLabels': ['feature', 'enhancement']}]
Starting the consolidation process.
Processing project with repository: living-doc-example-project...

Traceback (most recent call last):
File "/Users/ab024ll/repos/absa/develop/living-doc-generator/src/consolidate_feature_data.py", line 221, in
consolidated_features_with_project, set_of_used_repos = consolidate_features_with_project()
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/ab024ll/repos/absa/develop/living-doc-generator/src/consolidate_feature_data.py", line 141, in consolidate_features_with_project
merged_features = merge_feature_and_project_data(feature_data, project_data_dict, project_title)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/ab024ll/repos/absa/develop/living-doc-generator/src/consolidate_feature_data.py", line 75, in merge_feature_and_project_data
owner = feature['Owner']
~~~~~~~^^^^^^^^^
KeyError: 'Owner'
`

README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
src/controller.py Outdated Show resolved Hide resolved
src/controller.py Outdated Show resolved Hide resolved
src/utils.py Outdated Show resolved Hide resolved
src/github_query_issues.py Outdated Show resolved Hide resolved
src/github_query_issues.py Outdated Show resolved Hide resolved
src/github_query_issues.py Outdated Show resolved Hide resolved
src/github_query_issues.py Outdated Show resolved Hide resolved
src/containers/__init__.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@miroslavpojer miroslavpojer left a comment

Choose a reason for hiding this comment

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

  • pulled
  • code review

src/containers/__init__.py Outdated Show resolved Hide resolved
src/containers/__init__.py Outdated Show resolved Hide resolved
src/github_query_project_state.py Outdated Show resolved Hide resolved
src/github_query_project_state.py Outdated Show resolved Hide resolved
src/github_query_project_state.py Outdated Show resolved Hide resolved
src/github_query_project_state.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@miroslavpojer miroslavpojer left a comment

Choose a reason for hiding this comment

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

  • pulled
  • code review

src/clean_env_before_mining.py Show resolved Hide resolved
src/clean_env_before_mining.py Outdated Show resolved Hide resolved
src/clean_env_before_mining.py Show resolved Hide resolved
src/github_query_issues.py Show resolved Hide resolved
src/github_query_issues.py Outdated Show resolved Hide resolved
src/containers/issue.py Outdated Show resolved Hide resolved
src/containers/project.py Outdated Show resolved Hide resolved
src/containers/repository.py Outdated Show resolved Hide resolved
src/containers/issue.py Outdated Show resolved Hide resolved
src/containers/issue.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@miroslavpojer miroslavpojer left a comment

Choose a reason for hiding this comment

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

  • pulled
  • run
  • code review - skipped main scripts as so many changes noted in inner logic classes.

Notes to this review:

  • See comments calling for method renaming first.
    • There are so many load_from... types - I saw 3
  • See comments mentioning GithubManager.
    • In one of them is a link to a data example.
    • I would propose adopting this solution (later, we can create a commons repo)
  • Then, look for the rest of the comments.

Final note: Let's clean up a comment in this PR.

  • Create comment in issue parent Epic.
    • For all comments that are not OOP-related, create TODO notes with a short description.
    • Then, we can mark them as Resolved in this PR.

src/containers/base_container.py Outdated Show resolved Hide resolved
src/containers/consolidated_issue.py Outdated Show resolved Hide resolved
src/containers/consolidated_issue.py Outdated Show resolved Hide resolved
src/containers/consolidated_issue.py Outdated Show resolved Hide resolved
src/containers/repository.py Outdated Show resolved Hide resolved
src/containers/project_issue.py Outdated Show resolved Hide resolved
src/containers/project_issue.py Outdated Show resolved Hide resolved
src/utils.py Outdated Show resolved Hide resolved
src/utils.py Outdated Show resolved Hide resolved
src/utils.py Outdated Show resolved Hide resolved
@MobiTikula
Copy link
Collaborator Author

Release notes

  • implementing singleton GitHubManager for easier way to fetch data
  • Living doc project refactored into OOP
  • basic logging added to the project

@MobiTikula MobiTikula linked an issue Jul 16, 2024 that may be closed by this pull request
Copy link
Collaborator

@miroslavpojer miroslavpojer left a comment

Choose a reason for hiding this comment

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

  • pulled
  • run
  • code review

I will create two more refactoring issues. With them we can close this one as OOP only refactorin.

requirements.txt Outdated Show resolved Hide resolved
src/clean_env_before_mining.py Show resolved Hide resolved
src/github_query_issues.py Outdated Show resolved Hide resolved
src/github_query_project_state.py Outdated Show resolved Hide resolved
src/github_query_project_state.py Outdated Show resolved Hide resolved
src/consolidate_issue_data.py Outdated Show resolved Hide resolved
src/convert_issues_to_pages.py Outdated Show resolved Hide resolved
src/convert_issues_to_pages.py Outdated Show resolved Hide resolved
@MobiTikula MobiTikula merged commit 02ae6b7 into master Jul 17, 2024
3 checks passed
@MobiTikula MobiTikula deleted the feature/8_Refactoring_OOP_Data_containers branch July 17, 2024 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Improving code quality, paying off tech debt, aligning APIs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactoring - OOP: Data containers
2 participants