search

repobee / repobee-sanitizer

A plugin for sanitizing master repositories before distribution to students
MIT License
2 stars 3 forks source link

[fact] Refactor SanitizeRepo.command #139

Open slarse opened 3 years ago

slarse commented 3 years ago

@tohanss Here's the refactoring we looked at. I was gonna look it over again but I just didn't get around to it. Maybe you can make something of it.

Note that the core idea here was to have one form of error handling (here, we use exceptions). It would be equally valid to go with more monad-esque error handling and not throw exceptions. The important thing is to go with one kind.

codecov[bot] commented 3 years ago

Codecov Report

Merging #139 (2cfdabd) into master (9b5e599) will not change coverage. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #139   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            8        10    +2     
  Lines          255       285   +30     
  Branches        43        46    +3     
=========================================
+ Hits           255       285   +30
Impacted Files Coverage Δ
repobee_sanitizer/_errorutils.py 100.00% <100.00%> (ø)
repobee_sanitizer/_gitutils.py 100.00% <100.00%> (ø)
repobee_sanitizer/_sanitize_repo.py 100.00% <100.00%> (ø)
repobee_sanitizer/sanitizer.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update a7d648a...2cfdabd. Read the comment docs.

tohanss commented 3 years ago

Had to force push to remove an unrelated commit, no need to worry about that

tohanss commented 3 years ago

Based on the number of deleted lines on this PR id say sanitizer was due for refactoring, not to mention if we make a PR for SanitizeFile as well. Most added lines are just git not understanding some things that were changed to be shorter.

@slarse i will make a second check tomorrow with fresh eyes, otherwise, this is ready for you to look at whenever you feel like it

slarse commented 3 years ago

Will review this tonight.