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

Fix regex to allow underscores in job name. #640

Merged
merged 1 commit into from
Mar 10, 2020

Conversation

bcipriano
Copy link
Collaborator

Link the Issue(s) this Pull Request is related to.
Fixes #522

Summarize your change.
Underscores are allowed in job names, particularly the show and shot name; fix the regex to match.

This change resolves the leftover comments in #523: preserves . as an allowed character and adds some tests for Utils.findJob.

Copy link
Collaborator

@gregdenton gregdenton left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Contributor

@smith1511 smith1511 left a comment

Choose a reason for hiding this comment

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

Approved - I assume jobs can't have hyphens?

@bcipriano
Copy link
Collaborator Author

In my read of the code, hyphens in the actual "job name" are fine, but not in the show or shot names. When you submit a job the full final name becomes <show name>-<shot name>-<the actual job name>.

The regex here is only checking that convention and doesn't concern itself with what <the actual job name> is.

test_shouldSearchForJobByName verifies this logic -- there are some random symbols in the job name suffix but the test still passes ok.

@bcipriano bcipriano merged commit dae6524 into AcademySoftwareFoundation:master Mar 10, 2020
@bcipriano bcipriano deleted the fix-job-regex branch March 10, 2020 21:15
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.

find job regex is broken
3 participants
  翻译: