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 job.name unique requirement. #220

Merged
merged 2 commits into from
Jun 22, 2023
Merged

Remove job.name unique requirement. #220

merged 2 commits into from
Jun 22, 2023

Conversation

hulto
Copy link
Collaborator

@hulto hulto commented Jun 19, 2023

What type of PR is this?

/kind cleanup
/kind api-change

What this PR does / why we need it:

Remove the unique requirement from the job name.
For workflows like running shell scripts against a target I don't always want to update the jobs name.

Which issue(s) this PR fixes:

@hulto hulto added cleanup Code cleanup and tech debt removal tavern api-change labels Jun 19, 2023
@codecov
Copy link

codecov bot commented Jun 19, 2023

Codecov Report

Merging #220 (1615d05) into main (519d2fc) will decrease coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #220      +/-   ##
==========================================
- Coverage   72.30%   72.29%   -0.01%     
==========================================
  Files          82       82              
  Lines        4676     4675       -1     
==========================================
- Hits         3381     3380       -1     
  Misses       1210     1210              
  Partials       85       85              
Impacted Files Coverage Δ
tavern/ent/schema/job.go 34.78% <ø> (-1.39%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@Cictrone
Copy link
Collaborator

this will have to be an @KCarretto review as idk if he relies on job name being unique for any backend stuff.

@Cictrone
Copy link
Collaborator

Checked with @KCarretto and took a gander at the tavern code and looks like the answer is "no". At that point I think its just @cmp5987 who need to sign off on it.

@cmp5987 cmp5987 self-requested a review June 22, 2023 22:54
Copy link
Collaborator

@cmp5987 cmp5987 left a comment

Choose a reason for hiding this comment

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

Seems good to me. Based on discussions offline, we may want to have a the UI field have a default population based on tome + param details to reduce some of the frustration of writing descriptive job names.

@hulto hulto merged commit 5fa0273 into main Jun 22, 2023
8 checks passed
@hulto hulto deleted the non-unique-job-names branch June 22, 2023 22:57
KCarretto pushed a commit that referenced this pull request Feb 1, 2024
 
Remove job.name unique requirement. (#220)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-change cleanup Code cleanup and tech debt removal tavern
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants