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

Setup py writer #152

Merged
merged 8 commits into from
Dec 1, 2023
Merged

Setup py writer #152

merged 8 commits into from
Dec 1, 2023

Conversation

clavedeluna
Copy link
Contributor

@clavedeluna clavedeluna commented Nov 29, 2023

Overview

Integrate the setup.py writer to run and refactor

Description

  • We had a basic setup.py writer but it hadn't yet aligned with the other writers nor been enabled
  • Some refactors

Work we still need:

  • more robust file error handling for all parsers
  • implement the heuristic for picking which dependency file to use:

if there’s a pyproject.toml with dependencies, make updates there. Otherwise, if there’s a setup.py with dependencies, make the updates there. Finally, if neither of those are present, try to find requirements.txt and make the update there.

@clavedeluna
Copy link
Contributor Author

@drdavella I've addressed all your comments from previouis PR #150 (comment) in this PR

The only one that isn't as clean is refactoring the Change list. Since each writer has a tiny difference in how we get the line number. If you have a better idea lmk.

@clavedeluna clavedeluna force-pushed the setup-py-writer branch 2 times, most recently from e7efe0c to 9dfe421 Compare November 29, 2023 15:29
@drdavella
Copy link
Member

The only one that isn't as clean is refactoring the Change list. Since each writer has a tiny difference in how we get the line number. If you have a better idea lmk.

@clavedeluna would it be possible to pass the line number as a parameter to a factored out method? If so I think it's a worthwhile change because I really want to avoid having to make updates to this in multiple places.

@clavedeluna clavedeluna force-pushed the setup-py-writer branch 2 times, most recently from b5e39c8 to c9cbc0a Compare December 1, 2023 11:25
@clavedeluna clavedeluna marked this pull request as ready for review December 1, 2023 11:33
Copy link

codecov bot commented Dec 1, 2023

Codecov Report

Merging #152 (a5dd3b7) into main (575ed79) will increase coverage by 0.04%.
The diff coverage is 98.68%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #152      +/-   ##
==========================================
+ Coverage   96.43%   96.47%   +0.04%     
==========================================
  Files          74       74              
  Lines        3306     3351      +45     
==========================================
+ Hits         3188     3233      +45     
  Misses        118      118              
Files Coverage Δ
src/codemodder/codemodder.py 96.99% <100.00%> (-0.05%) ⬇️
src/codemodder/context.py 97.64% <100.00%> (ø)
...er/dependency_management/base_dependency_writer.py 96.87% <100.00%> (+0.32%) ⬆️
...demodder/dependency_management/pyproject_writer.py 100.00% <100.00%> (ø)
...r/dependency_management/requirements_txt_writer.py 100.00% <100.00%> (ø)
...odemodder/dependency_management/setup_py_writer.py 100.00% <100.00%> (ø)
src/codemodder/diff.py 93.54% <100.00%> (+0.69%) ⬆️
...odder/project_analysis/file_parsers/base_parser.py 100.00% <100.00%> (ø)
...der/project_analysis/file_parsers/package_store.py 100.00% <100.00%> (ø)
...nalysis/file_parsers/pyproject_toml_file_parser.py 100.00% <100.00%> (ø)
... and 4 more

@andrecsilva andrecsilva self-requested a review December 1, 2023 13:06
Copy link
Member

@drdavella drdavella left a comment

Choose a reason for hiding this comment

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

Good stuff 👍

@clavedeluna clavedeluna added this pull request to the merge queue Dec 1, 2023
Merged via the queue into main with commit 3b011b7 Dec 1, 2023
13 checks passed
@clavedeluna clavedeluna deleted the setup-py-writer branch December 1, 2023 16:58
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