Skip to content
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

Ignore schema_cache.dump on levelbuilder #21793

Merged
merged 2 commits into from Apr 11, 2018
Merged

Ignore schema_cache.dump on levelbuilder #21793

merged 2 commits into from Apr 11, 2018

Conversation

balderdash
Copy link
Contributor

I don't know why this file keeps getting changed. Until somebody can figure that out, tell content-push to ignore it.

Copy link
Member

@davidsbailey davidsbailey left a comment

Choose a reason for hiding this comment

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

This seems like a good short-term fix, given that it automates the process we're currently having developers do manually.

bin/content-push Outdated
@@ -5,6 +5,9 @@ require 'cdo/git_utils'
require 'cdo/only_one'

CONTENT_PATHS = 'dashboard pegasus aws/dms'.freeze
LEVELBUILDER_OMISSIONS = [
'dashboard/db/schema_cache.dump',
]
Copy link
Member

Choose a reason for hiding this comment

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

Please add a brief comment indicating that changes to the files in this list will be reverted during the content push, not just ignored.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was using git reset to just ignore them, but realized that would still create problems during the DTL step if there are schema_cache.dump changes. I added a git checkout to revert the changes, and comment to indicate that.

Copy link
Member

Choose a reason for hiding this comment

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

oops, I misread your git command, and was attempting to ask you to at least comment this rather harsh behavior. however you raise a good point that checkout is probably best here to prevent DTL problems, so what you have now looks good.

@@ -73,7 +86,8 @@ def commit_changes(name)

branchname = GitUtils.current_branch
exit 1 unless system("git add -A #{CONTENT_PATHS}")
exit 1 unless system("git commit #{CONTENT_PATHS} -m '#{branchname} content changes (-#{name})'")
ignore_levelbuilder_omissions
exit 1 unless system("git commit -m '#{branchname} content changes (-#{name})'")
Copy link
Member

Choose a reason for hiding this comment

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

just so that I understand, are you changing the behavior on this line or just removing an unnecessary but harmless #{CONTENT_PATHS}? the new syntax looks good, but I'm not sure I understand what the old syntax would do.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The old command ignored the files added by the git add on the previous line, and committed only the specified files (of course both commands used the same set of files, so it didn't matter). The new one commits everything that has been added, and ignores anything un-added by ignore_levelbuilder_omissions.

This change is no longer necessary now that I actually revert the changes rather than just resetting the file. I still think it's clearer this way though, since it's closer to the way most people use git add and git commit.

@balderdash balderdash merged commit 74f05fd into staging Apr 11, 2018
@balderdash balderdash deleted the scoop-filter branch April 12, 2018 17:42
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.

None yet

2 participants