Skip to content

build: in verbose mode, print what files we're syncing/where syncing them to [ch2130]#1463

Merged
dbentley merged 3 commits into
masterfrom
maiamcc/sync-transparency
Apr 11, 2019
Merged

build: in verbose mode, print what files we're syncing/where syncing them to [ch2130]#1463
dbentley merged 3 commits into
masterfrom
maiamcc/sync-transparency

Conversation

@maiamcc

@maiamcc maiamcc commented Apr 9, 2019

Copy link
Copy Markdown
Contributor

Hello @landism, @dbentley,

Please review the following commits I made in branch maiamcc/sync-transparency:

1e6abee (2019-04-09 15:45:33 -0400)
build: in verbose mode, print what files we're syncing/where syncing them to [ch2130]

Code review reminders, by giving a LGTM you attest that:

  • Commits are adequately tested
  • Code is easy to understand and conforms to style guides
  • Incomplete code is marked with TODOs
  • Code is suitably instrumented with logging and metrics

@maiamcc
maiamcc requested review from dbentley and landism April 9, 2019 19:45
@maiamcc
maiamcc force-pushed the maiamcc/sync-transparency branch from 1e6abee to 7130e90 Compare April 9, 2019 19:48
@maiamcc

maiamcc commented Apr 9, 2019

Copy link
Copy Markdown
Contributor Author

local container BaD:
Screen Shot 2019-04-09 at 3 44 01 PM

synclet BaD:
Screen Shot 2019-04-09 at 3 43 16 PM

Comment thread internal/build/container_updater.go Outdated
}

if len(toRemove) > 0 {
l.Verbosef("Deleting %d file(s) from container: %s", len(toRemove), cID.ShortStr())

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why verbose? This seems like good info to have in general. If the concern is length, I think there are ways to make it simpler.

In-container updating is fraught, and I'd rather overexplain (esp given comments we get like "I'm not sure sure what is happening or why")

@dbentley
dbentley merged commit 04499b5 into master Apr 11, 2019
@maiamcc
maiamcc deleted the maiamcc/sync-transparency branch June 21, 2019 18:17
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.

2 participants