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

Makefile: run tar and clean-tar targets on the CI #832

Closed
wants to merge 1 commit into from

Conversation

unclejack
Copy link
Contributor

This PR adds the tar and clean-tar targets to the CI. This ensures this test runs on the CI and would help catch potential issues with the tarballs which need to be released.

Signed-off-by: Cristian Staretu <cristian.staretu@gmail.com>
Copy link
Contributor

@dseevr dseevr left a comment

Choose a reason for hiding this comment

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

good idea

@jojimt
Copy link
Contributor

jojimt commented Apr 14, 2017

From what I can tell, all we are doing here is a fresh build & tar. Instead of running this as part of CI which already does "build", can we make this test part of the release script? That way, we will avoid adding more time to an already time-consuming CI.

@unclejack
Copy link
Contributor Author

@jojimt: The release target depends on the tar target and it calls clean-tar when it finishes running. This makes the release target a superset of this test.

You're right that it does add more to the execution time of the tests. Would you prefer that we make this change later on when we address the long execution of the tests? I don't have a strong opinion either way. I just wish to ensure everything gets tested as much as possible.

@jojimt
Copy link
Contributor

jojimt commented Apr 14, 2017

Perhaps you could add a verification to the release target. That should cover what you're trying to cover here.

@unclejack
Copy link
Contributor Author

This will require a different approach. I'll close this now.

@unclejack unclejack closed this Apr 21, 2017
@unclejack unclejack deleted the add_tar_targets_to_ci branch April 21, 2017 17:27
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

3 participants