Adds .tsuruignore file support #25

arxdsilva · 2017-02-08T13:35:45Z

andrestc · 2017-02-08T13:42:35Z

tsuru/client/deploy_test.go

+	c.Assert(headers, check.DeepEquals, expected)
+}
+
+func (s *S) TestIgnoreRelativeFolder(c *check.C) {


I'd replace folder with directory or dir

andrestc · 2017-02-08T13:43:01Z

tsuru/client/deploy_test.go

+	c.Assert(contents, check.IsNil)
+}
+
+func (s *S) TestIgnoreFolder(c *check.C) {


I'd replace folder with directory or dir

andrestc · 2017-02-08T13:44:01Z

tsuru/client/deploy_test.go

@@ -1,7 +1,3 @@
-// Copyright 2016 tsuru-client authors. All rights reserved.


I think this was removed by mistake, we need the header on every file.

andrestc · 2017-02-08T13:51:00Z

tsuru/client/deploy.go

@@ -290,13 +366,36 @@ func targz(ctx *cmd.Context, destination io.Writer, filepaths ...string) error {
 		if err != nil {
 			return err
 		}
+		wd, errWd := os.Getwd()
+		if errWd != nil {
+			return err


I think you mean't return errWd. I think you can also rename errWd to err

andrestc · 2017-02-08T13:54:13Z

tsuru/client/deploy.go

@@ -348,11 +447,38 @@ func addDir(writer *tar.Writer, dirpath string) error {
 	if err != nil {
 		return err
 	}
+	wd, errWd := os.Getwd()
+	if errWd != nil {
+		return err


Same as before

codecov-io · 2017-02-08T15:15:57Z

Codecov Report

Merging #25 into master will decrease coverage by -0.21%.

@@            Coverage Diff             @@
##           master      #25      +/-   ##
==========================================
- Coverage   67.54%   67.34%   -0.21%     
==========================================
  Files          34       34              
  Lines        6335     6403      +68     
==========================================
+ Hits         4279     4312      +33     
- Misses       1441     1464      +23     
- Partials      615      627      +12

Impacted Files	Coverage Δ
tsuru/client/deploy.go	`69.32% <53.94%> (-4.43%)`	❌

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 508e4bb...064e134. Read the comment docs.

andrestc · 2017-02-08T16:02:25Z

tsuru/client/deploy.go

+		if err != nil {
+			return err
+		}
+		fiName := filepath.Join(wd, fi.Name())
 		if fi.IsDir() {


I think lines 375-383 can be moved to before the if statement (so you can remove the ones in 389-397).

andrestc · 2017-02-08T16:21:11Z

I think there is some repetition that can be avoided by moving the checks to the begin of addDir and addFile.

cezarsa · 2017-02-08T16:03:43Z

tsuru/client/deploy.go

+		for _, pattern := range ignorePatterns {
+			ignPats, errProc := processTsuruIgnore(pattern, context.Args...)
+			if errProc != nil {
+				return err


return errProc

cezarsa · 2017-02-08T16:17:26Z

tsuru/client/deploy.go

+			return nil, err
+		}
+		for i := range paths {
+			if dir == "." {


Is this if necessary? Couldn't this loop always do paths[i] = filepath.Join(dir, paths[i])?

It is, we got this as a bug while uploading static into lab so without It, the path would be only paths[i] and It would never match glob on the root dir

cezarsa · 2017-02-08T16:18:16Z

tsuru/client/deploy.go

+		return nil, err
+	}
+	defer os.Chdir(old)
+	for _, dir := range dirPath {


It would be better to rename this vars to for _, dirPath := range dirPaths as dir is also used inside the for body.

cezarsa · 2017-02-08T16:19:17Z

tsuru/client/deploy.go

+			}
+			paths[i] = filepath.Join(dir, paths[i])
+		}
+		dir, err := os.Open(dir)


You need to call dir.Close() after the Readdir call.

cezarsa · 2017-02-08T16:21:50Z

tsuru/client/deploy.go

+	defer file.Close()
+	patterns := []string{}
+	scanner := bufio.NewScanner(file)
+	scanner.Split(bufio.ScanLines)


ScanLines is the default splitter, no need for this line.

cezarsa · 2017-02-08T16:23:07Z

tsuru/client/deploy.go

+	scanner.Split(bufio.ScanLines)
+	for scanner.Scan() {
+		pattern := scanner.Text()
+		patterns = append(patterns, pattern)


Simply patterns = append(patterns, scanner.Text())

cezarsa · 2017-02-08T16:27:27Z

tsuru/client/deploy.go

 		if fi.IsDir() {
+			for _, p := range ignoreList {


It would be more efficient for ignoreList to be a var ignoreSet map[string]struct{}. This way you could ignore patterns in O(1) by doing:

if _, inSet := ignoreSet[fiName]; !inSet { continue }

return errProc rename vars to `for _, dirPath := range dirPaths` call dir.Close() after the Readdir simplify scanner append

ignoreSet is a map[string]struct{} that allows better comparison to know If the pattern has matched or not

andrestc

LGTM

andrestc · 2017-02-13T17:24:43Z

This LGTM, what do you think, @cezarsa ?

Arthur Silva added 2 commits February 6, 2017 18:54

client: add .tsuruignore support

0e24550

client: fix ignore root directory files error

785a6b0

andrestc requested changes Feb 8, 2017

View reviewed changes

client: fix requested changes

bf7b4fe

andrestc reviewed Feb 8, 2017

View reviewed changes

client: fix error returns & add .tsuruignore to targz

517393e

client: fix tests & clear unnecessary checks

bc6c3f1

arxdsilva changed the title ~~Deploy ignore, solving tsuru/tsuru#1512~~ Adds .tsuruignore file support Feb 8, 2017

andrestc requested changes Feb 8, 2017

View reviewed changes

cezarsa requested changes Feb 8, 2017

View reviewed changes

Arthur Silva added 3 commits February 8, 2017 14:33

client: simplify lookup's & remove unnecessary import

6b4b15f

client: PR requested changes

f65f432

return errProc rename vars to `for _, dirPath := range dirPaths` call dir.Close() after the Readdir simplify scanner append

client: changes ignoreList to ignoreSet

064e134

ignoreSet is a map[string]struct{} that allows better comparison to know If the pattern has matched or not

andrestc approved these changes Feb 9, 2017

View reviewed changes

cezarsa merged commit d7c3712 into tsuru:master Feb 13, 2017

arxdsilva deleted the deploy-ignore branch February 13, 2017 17:36

keydrevolta mentioned this pull request Oct 29, 2017

Exclude option support hairyhenderson/gomplate#218

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds .tsuruignore file support #25

Adds .tsuruignore file support #25

arxdsilva commented Feb 8, 2017 •

edited

Loading

andrestc Feb 8, 2017

andrestc Feb 8, 2017 •

edited

Loading

andrestc Feb 8, 2017

andrestc Feb 8, 2017

andrestc Feb 8, 2017

codecov-io commented Feb 8, 2017 •

edited

Loading

andrestc Feb 8, 2017

andrestc commented Feb 8, 2017

cezarsa Feb 8, 2017

cezarsa Feb 8, 2017

arxdsilva Feb 8, 2017 •

edited

Loading

cezarsa Feb 8, 2017

cezarsa Feb 8, 2017

cezarsa Feb 8, 2017

cezarsa Feb 8, 2017

cezarsa Feb 8, 2017

andrestc left a comment

andrestc commented Feb 13, 2017

		@@ -1,7 +1,3 @@
		// Copyright 2016 tsuru-client authors. All rights reserved.

Adds .tsuruignore file support #25

Adds .tsuruignore file support #25

Conversation

arxdsilva commented Feb 8, 2017 • edited Loading

Choose a reason for hiding this comment

andrestc Feb 8, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented Feb 8, 2017 • edited Loading

Codecov Report

Choose a reason for hiding this comment

andrestc commented Feb 8, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arxdsilva Feb 8, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrestc left a comment

Choose a reason for hiding this comment

andrestc commented Feb 13, 2017

arxdsilva commented Feb 8, 2017 •

edited

Loading

andrestc Feb 8, 2017 •

edited

Loading

codecov-io commented Feb 8, 2017 •

edited

Loading

arxdsilva Feb 8, 2017 •

edited

Loading