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

Add release_tag to config checklist #161

Merged
merged 10 commits into from
May 23, 2024
Merged

Conversation

sjspielman
Copy link
Member

Does what it says! I realized this was missing from the file.

Worth noting this is the first time we're doing a workshop with the updated version of training-specific-template so there may be more of these small fixes. Please have a quick look at the config to check that nothing else is missing too - I think we're good now?

@sjspielman sjspielman requested a review from jashapiro May 20, 2024 20:06
Copy link
Member

@jashapiro jashapiro left a comment

Choose a reason for hiding this comment

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

I think this may be intentionally not in the list, as we don't update this until much later? The docker tag also probably doesn't get updated yet though...

I think the better thing to do might be to add the docker tag to https://github.com/AlexsLemonade/training-specific-template/blob/main/setup-issue-templates/workshop-release.md?

We will also want to note there (or somewhere) that creating the tag will build a new docker image with a matching tag (this is automatic now!). Which may mean we don't really need a separate docker tag, but it is probably worth leaving them separate for now.

@sjspielman
Copy link
Member Author

I think the better thing to do might be to add the docker tag to https://github.com/AlexsLemonade/training-specific-template/blob/main/setup-issue-templates/workshop-release.md?

This makes sense to me

@sjspielman
Copy link
Member Author

@jashapiro is this about what you had in mind?

@sjspielman sjspielman requested a review from jashapiro May 22, 2024 21:02
@sjspielman sjspielman requested a review from jashapiro May 23, 2024 12:44
Copy link
Member

@jashapiro jashapiro left a comment

Choose a reason for hiding this comment

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

Could you also update the default in

docker_repo: DOCKER-REPOSITORY
docker_tag: DOCKER-TAG

@sjspielman
Copy link
Member Author

Could you also update the default in

@jashapiro Did you want a default of edge or similar for docker_tag?

@jashapiro
Copy link
Member

Could you also update the default in

@jashapiro Did you want a default of edge or similar for docker_tag?

You could. I don't think it matters too much.

@sjspielman
Copy link
Member Author

You could. I don't think it matters too much.

I did it for the pun. Worth it.

@sjspielman sjspielman merged commit f434a0e into main May 23, 2024
1 check passed
@sjspielman sjspielman deleted the sjspielman/missed-a-config branch May 23, 2024 15:05
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