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

fix: handle os.ErrExist on subsequent reruns #30

Closed
wants to merge 7 commits into from

Conversation

zalgonoise
Copy link

@zalgonoise zalgonoise commented Mar 31, 2024

This PR fixes an issue reported by other users a while ago on #21, which pointed out that running the same executable / script several times (e.g. when changing or updating the same diagram) results in os.ErrExist errors considering that the go-diagrams and inner assets folders already existed.

This is a known error returned in os.MkDir and os.MkDirAll that can be checked with an (optionally in-line) errors.Is call.

These changes are applied to the diagram and cmd/gen package, where (black-box) tests were added to the diagram package, and the assets regenerated (with go run ./cmd/gen).

This PR also updates all dependencies to their latest versions; removes the usage of ioutil (as a deprecated package, if favor of os functions); and regenerates all assets. The later is an important step to mention since it seems these were generated in a different order in some symbols. I am unsure of the reason why (didn't look into that) however I can also remove the assets folder commit in particular, if you wish to skip changes to that package. I believe it's always a good idea to regen on changes, but there is a context to it too.

There are other details that I see potential in improvement, and I wish to contribute to this repository, but all in their due PRs and topics :D these changes should only comprehend the issue for the existing files when re-rendering a diagram.

@zalgonoise
Copy link
Author

I see that other users have already posted similar (and simpler) PRs for this issue. I will fork the repository and continue working on it for my own requirements :) You can ignore this PR, or re-open it / mention me if you'd like to apply a patch to your repository with any of the content

@zalgonoise zalgonoise closed this Apr 2, 2024
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.

1 participant