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

Use POSIX ERE for sub function #321

Merged
merged 3 commits into from
Jan 31, 2020
Merged

Conversation

DavyCats
Copy link
Contributor

@DavyCats DavyCats commented Jan 7, 2020

Motivation

This PR implements openwdl/wdl#243

Approach

Uses the POSIX implementation of the regex package.

Checklist

  • Add appropriate test(s) to the automatic suite
  • Use make pretty to reformat the code with black
  • Use make check to statically check the code using Pyre and Pylint
  • Send PR from a dedicated branch without unrelated edits
  • Ensure compatibility with this project's MIT license

@coveralls
Copy link

coveralls commented Jan 7, 2020

Pull Request Test Coverage Report for Build 1487

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 96.804%

Totals Coverage Status
Change from base Build 1483: 0.0%
Covered Lines: 4846
Relevant Lines: 5006

💛 - Coveralls

@DavyCats
Copy link
Contributor Author

There was an error in the linting, because there was no stub for the regex module. I added one, though I'm not 100% sure this is the correct way of doing this. Please let me know if something is amiss.

@mlin
Copy link
Collaborator

mlin commented Jan 17, 2020

@DavyCats thx & sorry for slow uptake on this, I've been tied up moving apartments. Will help with the hygiene obstacles

@DavyCats
Copy link
Contributor Author

@mlin No problem, there's no haste behind this whatsoever, so if you don't have the time right now it can wait.

@mlin mlin merged commit 1e59170 into chanzuckerberg:master Jan 31, 2020
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.

3 participants