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

Move removal of files to clean #3726

Merged
merged 2 commits into from Mar 28, 2017
Merged

Move removal of files to clean #3726

merged 2 commits into from Mar 28, 2017

Conversation

ruflin
Copy link
Member

@ruflin ruflin commented Mar 6, 2017

This makes sure all generated files are removed on clean and not before they are regenerated.

@ruflin ruflin added the review label Mar 6, 2017
This makes sure all generated files are removed on clean and not before they are regenerated.
rm -rf _meta/kibana/dashboard _meta/kibana/search _meta/kibana/visualization
# Clean module docs
rm -rf docs/modules

# Collects all module dashboards
.PHONY: kibana
kibana:
Copy link
Contributor

Choose a reason for hiding this comment

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

Should then kibana depend on clean? Otherwise I'm afraid we might get old dashboards mixed with the new ones.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm kind of undecided on this PR now looking at it again. It's nice that all clean is one place but at the same time it then requires kibana to depend on clean which I don't like. We should not get old dashboards because of CI always starts on a fresh build and would detect the old files.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, yeah, still feels risky to me. Maybe separate a kibana-clean target on which both clean and kibana depend?

@ruflin
Copy link
Member Author

ruflin commented Mar 27, 2017

@tsg I went for duplicating the code instead of introducing an additional target. The important part in this PR is that clean: removes all the temp files. It's ok if some of the targest to also some partial cleanup itself.

@tsg tsg merged commit e6c0f45 into elastic:master Mar 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants