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

Remove step to push image to repository. #79

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

StevenYCChou
Copy link
Contributor

No description provided.

@@ -29,9 +29,6 @@ git checkout -b "${versioned_release_branch}"
git add VERSION
git commit -m "Update version to ${version}."

# 4. Run `DOCKER_IMAGE_NAME={public_docker_image} make push`.
DOCKER_IMAGE_NAME="gcr.io/stackdriver-prometheus/stackdriver-prometheus-sidecar" make push
Copy link
Contributor

Choose a reason for hiding this comment

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

With this change, this script doesn't really release any artifacts anymore. Please provide more information in the PR description and update RELEASE-PROCESS.md to include this manual step and explain why one wouldn't want to normally execute it, which is the same reason why you are removing it from this file.

Copy link
Contributor

Choose a reason for hiding this comment

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

Since we are moving the release process to internal only, do we still need the release.sh and RELEASE-PROCESS.md in the OSS world? I'd propose just deleting them. When the time comes when we want to automate everything (tagging and publish release notes), we can refer to these files (they will be in Git history).

Copy link
Contributor

Choose a reason for hiding this comment

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

As long as the internal release process takes care of tagging, that's fine with me. We do need to document how to run the end-to-end test while this is an OSS project.

Copy link
Contributor

Choose a reason for hiding this comment

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

Currently tagging is not taken care of. To fully integrate with the internal build infra, we need something like #80.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sync'ed with Javier offline. We will need two versions of release script, one for internal and one for external use case where ppl can build and host an image on their own. Please ignore the comment above. I've sent a PR internally for the internal release.sh script.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My understanding are the following statements:

  • our team will publish tagged image with internal script.
  • As it's an OSS project, we'd want to provide users instructions to:
    • build image,
    • run end-to-end test, and
    • push image to user owned image repository.

If the above statements are correctly, I will make the corresponding modification.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, that makes sense to me.

Copy link
Contributor

@qingling128 qingling128 left a comment

Choose a reason for hiding this comment

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

The image push has been moved to internal release process. This change LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants