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

cylc validate: fix failure on XXX-FAM task name #1779

Merged

Conversation

matthewrmshin
Copy link
Contributor

Close #1778.

@matthewrmshin matthewrmshin added the bug Something is wrong :( label Apr 6, 2016
@matthewrmshin matthewrmshin added this to the next-release milestone Apr 6, 2016
@matthewrmshin
Copy link
Contributor Author

@hjoliver @arjclark please review.

@@ -1400,7 +1400,12 @@ def process_graph_line(self, line, section, seq, offset_seq_map,
# backslashed re markers like \b from being interpreted as
# normal escapeded characters.

if fam not in line:
fam_in_line = re.search(
r"(?<!%(name_char_re)s)%(fam)s(?!(name_char_re)s)" % {
Copy link
Member

Choose a reason for hiding this comment

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

Missing '%' in the second name_char_re string template!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, and added extra test.

@matthewrmshin matthewrmshin force-pushed the fix-validate-hyphen-fam-name-failure branch from 107d2a8 to 47cca5a Compare April 7, 2016 08:20
@matthewrmshin
Copy link
Contributor Author

Branch re-based.

@hjoliver
Copy link
Member

hjoliver commented Apr 7, 2016

Review 1 - good, tests pass.

@hjoliver hjoliver assigned arjclark and unassigned hjoliver Apr 7, 2016
@arjclark
Copy link
Contributor

arjclark commented Apr 8, 2016

Looks good to me. Test battery passing in my environment as is manual testing with the example in #1778.

@arjclark arjclark merged commit 3542aee into cylc:master Apr 8, 2016
@matthewrmshin matthewrmshin deleted the fix-validate-hyphen-fam-name-failure branch April 8, 2016 13:39
matthewrmshin added a commit to matthewrmshin/cylc-flow that referenced this pull request Apr 27, 2016
It was attempted in cylc#1779 but the fix had a speed issue. This attempt
improves the speed and reduces the complexity of the original fix.
matthewrmshin added a commit to matthewrmshin/cylc-flow that referenced this pull request Apr 27, 2016
It was attempted in cylc#1779 but the fix had a speed issue. This attempt
improves the speed and reduces the complexity of the original fix.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is wrong :(
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants