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.

Sign up for GitHub

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

Jump to bottom

Consistent job naming #463

Merged
merged 1 commit into from
Jul 30, 2021
Merged

Consistent job naming #463

merged 1 commit into from
Jul 30, 2021

Conversation

bastjan
Copy link
Contributor

@bastjan bastjan commented Jul 23, 2021 •
edited

Summary

This PR:

  • Prefixes the job with the backup type
  • Sets the type as the container name

Part of #427.

Checklist

  • Keep pull requests small so they can be easily reviewed.
  • Categorize the PR by setting a good title and adding one of the labels:
    bug, enhancement, documentation, change, breaking,
    as they show up in the changelog
  • Update the documentation. (Looked through the docs, the generated batch/v1.Job name is referenced nowhere)
  • Update tests.
  • Link this PR to related issues.

@bastjan bastjan changed the base branch from improve-monitoring to master July 23, 2021 08:59
@bastjan bastjan force-pushed the consistent-container-names branch 3 times, most recently from 214e205 to 1a06645 Compare July 28, 2021 09:20
@bastjan bastjan added the change Generic change that is neither a fix or feature label Jul 28, 2021
@bastjan bastjan force-pushed the consistent-container-names branch from 1a06645 to f710622 Compare July 28, 2021 09:32
@bastjan bastjan marked this pull request as ready for review July 28, 2021 09:52
ccremer
ccremer reviewed Jul 28, 2021
View reviewed changes
job/job.go Outdated Show resolved Hide resolved
api/v1alpha1/schedule_types.go Outdated Show resolved Hide resolved
corvus-ch
corvus-ch approved these changes Jul 29, 2021
View reviewed changes
Copy link
Contributor

@corvus-ch corvus-ch left a comment

Choose a reason for hiding this comment

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

Whith the comments by @ccremer addressed, this is LGTM.

api/v1alpha1/job_name_test.go Outdated Show resolved Hide resolved
@bastjan bastjan requested a review from ccremer July 30, 2021 15:22
@bastjan bastjan force-pushed the consistent-container-names branch from fec5866 to c31e4b2 Compare July 30, 2021 15:36
@bastjan
Consistent job naming
9bbe92b 
Prefix job with backup type and use the type for the container name.
@bastjan bastjan force-pushed the consistent-container-names branch from c31e4b2 to 9bbe92b Compare July 30, 2021 15:43
@bastjan bastjan merged commit 7e82781 into master Jul 30, 2021
@bastjan bastjan deleted the consistent-container-names branch July 30, 2021 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@corvus-ch corvus-ch corvus-ch approved these changes

@ccremer ccremer ccremer approved these changes

@cimnine cimnine Awaiting requested review from cimnine

Assignees
No one assigned
Labels
change Generic change that is neither a fix or feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@bastjan @corvus-ch @ccremer