Skip to content
This repository has been archived by the owner on Apr 3, 2024. It is now read-only.

Fix bugs in findScripts #223

Merged
merged 1 commit into from
Jan 20, 2017
Merged

Fix bugs in findScripts #223

merged 1 commit into from
Jan 20, 2017

Conversation

ofrobots
Copy link
Contributor

  • Code was buggy when fileStats had only one entry.
  • Fix bug in pathToRegExp that was not considering '.' as literal.
  • Make the code easier to unit test.
  • More unit tests.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jan 20, 2017
@ofrobots
Copy link
Contributor Author

PTAL.

@ofrobots ofrobots force-pushed the findScripts branch 2 times, most recently from 90b2ecd to e27eb52 Compare January 20, 2017 16:36
Copy link
Contributor

@matthewloring matthewloring left a comment

Choose a reason for hiding this comment

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

LGTM with nits


/**
* Given an list available files and a script path to match, this function tries
* to resolve the script patch a (hopefully unique) match in the file list

This comment was marked as spam.

This comment was marked as spam.

/**
* Given an list available files and a script path to match, this function tries
* to resolve the script patch a (hopefully unique) match in the file list
* irrespective of the script path. This can be useful because repo file paths

This comment was marked as spam.

This comment was marked as spam.

* Code was buggy when fileList had only one entry.
* Fix bug in pathToRegExp that was not considering '.' as literal.
* Make the code easier to unit test.
* More unit tests.
@ofrobots ofrobots merged commit 04103e5 into googleapis:master Jan 20, 2017
@ofrobots ofrobots deleted the findScripts branch January 20, 2017 21:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants