Skip to content
This repository has been archived by the owner on Mar 25, 2024. It is now read-only.

Support exclude patterns #55

Closed
wants to merge 2 commits into from
Closed

Conversation

karalabe
Copy link
Contributor

This PR contains an implementation of exclude patterns, where the user may specify certain packages )or folders) to exclude from vendoring. For the rationale behind this please see #54.

The exclusion pattern in the plain config file format is an import path with an dash (-) preceding it, so it's unambiguous and backwards compatible with the current mode of operation. In yaml format a new exclude field is used which contains a list of paths to exclude. If an exclude pattern is present in the config file, trash will first delete those from any vendored packages and only then proceed with the removeUnusedImports step, ensuring that anything that only excluded packages depended on is also removed.


Beside the above feature, the PR contains two bugfixes:

Previously a trash --update overwrote any existing config file without retaining its original format (#46). This had the unfortunate side effect that an original yaml config file was swapped out with a flat conf type. A proposal to fix this was done in #50, but adding a new flag is a redundant operation since the code already knows what the original format was. This PR ensures that the original config file format (yaml or conf) is retained inside the Conf struct, allowing to serialize back to the same format on a dump.

Another bug was that trash recreated a new config structure from scratch on update, overwriting the old struct before the package metadata (version, external repo) could be extracted, losing all such information. A proposal was opened in #51 to fix this, but it doesn't play nice with other internal/private fields in the Conf struct. This PR offers an alternative solution by not recreating the original config struct, just its Imports field. This was all private metadata is preserved, including the importMap containing the import meta fields.

@imikushin
Copy link
Contributor

Can you separate the bug fixes from #54 implementation? Let's keep things as unentangled as possible :)

Anyways, thanks a lot for using and contributing, @karalabe !

@karalabe
Copy link
Contributor Author

Thing is, both the yaml fix and the exclusion patters rely on fixing the metadata bug, and fixing it in this particular way opposed to the other currently open PR. Hence why I did a combo pull. If you want I can split them up, but that will require multiple rounds of merges.

@karalabe
Copy link
Contributor Author

Meh, here's the bugfix PR :) #56

@karalabe
Copy link
Contributor Author

I've also updated this PR to have the previous bugfix commit and a separate commit for the exclusion patterns. Should allow merging both PRs without conflicts :)

@karalabe karalabe changed the title Support exclude patterns, keep conf format, retain old metadata Support exclude patterns Oct 28, 2016
@imikushin
Copy link
Contributor

@karalabe I've had to rebase this PR's commit before I could merge it (my fault, sorry!). Good thing, the change is actually merged into master now.

I'm going to tag a new release shortly.

Thanks a ton for your contributions and patience! 🍺

@imikushin imikushin closed this Nov 4, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants