macports / upt-macports

The Universal Packaging Tool (upt) backend for MacPorts
https://framagit.org/upt/upt
BSD 3-Clause "New" or "Revised" License
9 stars 12 forks source link

Create portfile first and then write to file or output #48

Closed Korusuke closed 5 years ago

Korusuke commented 5 years ago

Fixes #42

reneeotten commented 5 years ago

thanks @Korusuke: this does solve the problem I mentioned by first generating the Portfile and, only if that works, to create the output directory and write it to a file.

I think it's fine to have an extra variable portfile and use that as an argument for the __create_portfile function, but there might be other ways as well. I am sure @Steap has some insight here on what is most Pythonic and/or he prefers!

Korusuke commented 5 years ago

ping @Steap I guess this PR can be merged now?

codecov[bot] commented 5 years ago

Codecov Report

Merging #48 into master will increase coverage by 1.14%. The diff coverage is 60%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #48      +/-   ##
==========================================
+ Coverage   86.48%   87.63%   +1.14%     
==========================================
  Files           1        1              
  Lines         185      186       +1     
  Branches       10       10              
==========================================
+ Hits          160      163       +3     
+ Misses         25       23       -2
Impacted Files Coverage Δ
upt_macports/upt_macports.py 87.63% <60%> (+1.14%) :arrow_up:

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 f3c01cf...5a9ab29. Read the comment docs.

Steap commented 5 years ago

ping @Steap I guess this PR can be merged now?

Please squash the commits :) You may also want to add "Closes #42" at the end of the commit message.

Otherwise, looks good to me. @reneeotten Once Korusuke has squashed the commits, can you test this PR and merge it if it solves your issue?