Skip to content

Support ignored files as a parameter #77

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

Merged
merged 2 commits into from
Jul 9, 2020

Conversation

erikh
Copy link
Contributor

@erikh erikh commented Jul 7, 2020

This closes #74, see comments for design work.

Still unfinished as of this writing.

@erikh erikh force-pushed the ignore-lists branch 2 times, most recently from e6e5d5d to 84af74e Compare July 7, 2020 16:07
@erikh
Copy link
Contributor Author

erikh commented Jul 7, 2020

This has been changed to live entirely in the spec, as requested.

There are two fields, the "string form" (which is easier for end-users in many cases) and the "list form" (the more programmable version). I figured both of these would be nice, but feel free to tell me otherwise. I would vastly prefer the list form if I had to choose.

There's still some docs and testing to do but this is functioning as intended with initial hand testing.

@erikh
Copy link
Contributor Author

erikh commented Jul 7, 2020

Another thing we could do, is coalesce these into a subtype that is used by the spec.

It would also make the logic simpler in the loadExcludedFiles code by keying off a single attribute.

LMK how you'd like me to press forward.

@stefanprodan
Copy link
Member

@erikh i would opt for a single field that represents a .sourceignore file embedded like showed in the issue, with one expression per line.

@erikh
Copy link
Contributor Author

erikh commented Jul 8, 2020

Adjusted to support .sourceignore format, PTAL at impl, I think you'll be happy with this round.

Will hack on some tests tomorrow hopefully.

@erikh erikh force-pushed the ignore-lists branch 3 times, most recently from 79a47de to ae3d7b9 Compare July 8, 2020 14:37
-- More coming in this commit message soon

Signed-off-by: Erik Hollensbe <github@hollensbe.org>
@erikh
Copy link
Contributor Author

erikh commented Jul 8, 2020

ok, sorry about that, some messy comments from the earlier patches. I've also adjusted a few small bits around the handling of the .gitignore scanner that simplifies things since I made the last comment.

Should be reviewable now.

@stefanprodan
Copy link
Member

I just noticed that the defaults are broken in master, I'm getting the .git dir in the artifact. Tests would be awesome

@stefanprodan
Copy link
Member

The API looks good now, thanks!

@erikh
Copy link
Contributor Author

erikh commented Jul 8, 2020

I can't reproduce (the git directory is not in the tarball), but I will try to make a test for that separately.

@stefanprodan
Copy link
Member

I can't reproduce (the git directory is not in the tarball), but I will try to make a test for that separately.

I had a .sourceignore file in one of my test repo 🤦 all good, my messy tests were at fault

@erikh erikh force-pushed the ignore-lists branch 2 times, most recently from d9c2fdd to bb1a73e Compare July 8, 2020 23:21
Signed-off-by: Erik Hollensbe <github@hollensbe.org>
@erikh erikh changed the title [WIP] Support ignored files as a parameter Support ignored files as a parameter Jul 8, 2020
@erikh
Copy link
Contributor Author

erikh commented Jul 8, 2020

This is ready. PTAL please.

Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Thanks @erikh

@stefanprodan stefanprodan merged commit 74c97ad into fluxcd:master Jul 9, 2020
@stefanprodan stefanprodan added the area/git Git related issues and pull requests label Jul 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/git Git related issues and pull requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature requests around CRD API WRT file inclusion/exclusion
2 participants