jfeliu007 / goplantuml

PlantUML Class Diagram Generator for golang projects
MIT License
1.78k stars 167 forks source link

Init go modules #88

Closed ferhatelmas closed 2 years ago

ferhatelmas commented 4 years ago

Move to go modules with v2 and update readme and travis accordingly.

Bump supported go versions from go1.11 to latest.

Resolves #31.

codecov[bot] commented 4 years ago

Codecov Report

Merging #88 (8563ce7) into master (383ece0) will decrease coverage by 0.39%. The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##            master      #88      +/-   ##
===========================================
- Coverage   100.00%   99.60%   -0.40%     
===========================================
  Files            5        5              
  Lines          510      510              
===========================================
- Hits           510      508       -2     
- Misses           0        1       +1     
- Partials         0        1       +1     
Impacted Files Coverage Δ
parser/class_parser.go 99.43% <0.00%> (-0.57%) :arrow_down:

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 383ece0...8563ce7. Read the comment docs.

jfeliu007 commented 4 years ago

Thanks for the this PR. I didn't work on this yet because I am still green on the area of Go modules. Would you mind waiting a bit until I have the time to read more on go modules before we merge this one? There is a portion of the testing that is not being covered in this PR and I would like to see if we can remove the code or if we need to provide testing for that. :)

ferhatelmas commented 4 years ago

Sure, no worries. Take your time and anything else is needed, let me know.

StacieClark commented 3 years ago

What is the status of this?

jfeliu007 commented 3 years ago

Failing tests. This might need to be redone. The test failed because the vendor folder was removed. We need to either change the test or include the vendor folders in the modules as part of the module package.

ferhatelmas commented 3 years ago

I can update it if you want to take it in.

jfeliu007 commented 3 years ago

sure. master has quite a few changes since then. Make sure to take it into account. I would like to maintain the vendors' folder with go mode vendors and update the test if necessary. Thank you very much.

ferhatelmas commented 3 years ago

@jfeliu007 questions?

  1. do you want to bump go versions from go 1.14 ? (IMHO why not)
  2. do you want to keep testingsupport/vendor folder ? (IMHO no reason)
jfeliu007 commented 3 years ago

@jfeliu007 questions?

  1. do you want to bump go versions from go 1.14 ? (IMHO why not)
  2. do you want to keep testingsupport/vendor folder ? (IMHO no reason)

1.- The lowest version necessary to support mods (I think it is 1.11, right) (Unless we are using a feature from a later version with this update) 2.- The purpose of the vendor folder in the testingsupport folder is to test that the library skips the vendors folder. There is a test for that. We either need to remove the test for the vendors folder or keep the vendors folder in the testingsupport folder.

I am concerned now that automatic testing is not happening. Let me see if I can fix that before merging this.

ferhatelmas commented 3 years ago

@jfeliu007 Thanks for the details, I will address both.

mariotoffia commented 3 years ago

@jfeliu007 It would be super nice to have this PR going through! :)

ferhatelmas commented 3 years ago

@jfeliu007 addressed your comments and summary of changes:

Also, good to move to github actions IMHO, can take care of this next if you wish.

mariotoffia commented 3 years ago

@jfeliu007 Is there anything more needed in order to get @ferhatelmas PR merged?

Cheers, Mario :)

icholy commented 3 years ago

There is no reason to bump the version to v2.

ferhatelmas commented 2 years ago

going over open PRs from me without any updates and closing, so let me know if anything is needed from me.