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

implement file ignore in the profile enable process #104

Merged
merged 1 commit into from
Oct 29, 2019

Conversation

chanwit
Copy link
Member

@chanwit chanwit commented Oct 25, 2019

This PR makes the profile enable command to uses contents of the .wksctlignore file, if exists on the top most directory of the profile, to filter out unneeded files.

What does this PR do:

  1. detect .wksctlignore at the top level directory of the profile.
  2. use each line of the .wksctlignore file to do git rm.
  3. amend the previous commit (done by git subtree add)

A test case to test the processing of .wksctlignore is included.

@chanwit chanwit force-pushed the support_ignore_file branch 5 times, most recently from 6fd56e3 to 2067257 Compare October 27, 2019 18:43
@chanwit chanwit marked this pull request as ready for review October 27, 2019 19:35
@chanwit
Copy link
Member Author

chanwit commented Oct 27, 2019

cc @palemtnrider
This is the PR to process the ignore file for wskctl.

AppDevRepoURL = "git@github.com:weaveworks/eks-quickstart-app-dev"
AppDevAlias = "app-dev"
AppDevRepoURL = "git@github.com:weaveworks/eks-quickstart-app-dev"
WKSctlIgnoreFilename = ".wksctlignore"
Copy link
Contributor

Choose a reason for hiding this comment

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

It feels to me that wksctl in wksctlignore refers to our tool in an overly specific way. It is not impossible that we have another tool (other than wksctl) in the future that deals with profiles, for example.

Unless we're already committed to calling this .wksctlignore, I suggest renaming to .profileignore.

Copy link
Member Author

Choose a reason for hiding this comment

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

@mflendrich basically yes, it is very specific to wksctl. Let me explain by referring to this example profile:
https://github.com/chanwit/eks-quickstart-app-dev

In the above profile, there are two ignore files .eksctlignore and .wksctlignore.
Each of them contains a set of file needs to be removed by each tool.

This is by design to make a single profile repo works on both eksctl and wksctl.
Thinking of this as a situation that we have .circleci, .travis.yaml and .drone.yaml files on a git repo to allow each CI to independently process our repo (in its own way).

cmd/wksctl/profile/constants/constants.go Show resolved Hide resolved
cmd/wksctl/profile/enable/enable.go Outdated Show resolved Hide resolved
cmd/wksctl/profile/enable/enable.go Outdated Show resolved Hide resolved
cmd/wksctl/profile/enable/enable.go Show resolved Hide resolved
cmd/wksctl/profile/enable/enable.go Outdated Show resolved Hide resolved
cmd/wksctl/profile/enable/enable.go Outdated Show resolved Hide resolved
pkg/git/git.go Outdated Show resolved Hide resolved
@chanwit chanwit force-pushed the support_ignore_file branch 2 times, most recently from 868d20b to 678de73 Compare October 28, 2019 09:47
Copy link
Contributor

@fbarl fbarl left a comment

Choose a reason for hiding this comment

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

Very nice and clean PR, code LGTM! 👍

@chanwit chanwit merged commit dbf21d2 into master Oct 29, 2019
@chanwit
Copy link
Member Author

chanwit commented Oct 29, 2019

Thank you @mflendrich and @fbarl for your kindly reviews!

@chanwit chanwit deleted the support_ignore_file branch April 29, 2020 02:05
D3nn pushed a commit that referenced this pull request May 8, 2020
implement file ignore in the profile enable process
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.

None yet

3 participants