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.

Sign up for GitHub

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

Jump to bottom

Export parseQuery (#76) #77

Merged
merged 4 commits into from
Mar 16, 2017
Merged

Export parseQuery (#76) #77

merged 4 commits into from
Mar 16, 2017

Conversation

anseki
Copy link
Contributor

@anseki anseki commented Mar 14, 2017

  • Add exports.parseQuery in lib/index.js
  • Separate test/parseQuery.test.js from test/getOptions.test.js
  • Add parseQuery in README.md

@anseki
Export parseQuery (webpack#76)
e0a12cc 
- Add `exports.parseQuery` in `lib/index.js`
- Separate `test/parseQuery.test.js` from `test/getOptions.test.js`
- Add `parseQuery` in `README.md`
@jsf-clabot
Copy link

jsf-clabot commented Mar 14, 2017 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

jhnns
jhnns requested changes Mar 15, 2017
View reviewed changes
Copy link
Member

@jhnns jhnns left a comment

Choose a reason for hiding this comment

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

Awesome 👍

There are only three small nitpicks, sorry :). After that, we're good to go.

test/getOptions.test.js Outdated
test.expected
);
});
describe("when loaderContext.query is a string that has length", () => {
Copy link
Member

Choose a reason for hiding this comment

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

I'd write ... is a string with length > 0

README.md Outdated
Parses a passed string (e.g. `loaderContext.resourceQuery`) as a query string, and returns an object.

``` javascript
var params = loaderUtils.parseQuery(this.resourceQuery); // resource: `file?param1=foo`
Copy link
Member

Choose a reason for hiding this comment

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

Please use const in the README if the value is not changed

README.md Outdated

``` javascript
var params = loaderUtils.parseQuery(this.resourceQuery); // resource: `file?param1=foo`
if (params.param1 === 'foo') {
Copy link
Member

Choose a reason for hiding this comment

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

Please use double quotes

@anseki
Copy link
Contributor Author

anseki commented Mar 15, 2017 •
edited
Loading

I did not change other vars in other parts, e.g. urlToRequest.
Should I change those also?

@jhnns
Copy link
Member

jhnns commented Mar 16, 2017

Oh, that would be great. Didn't realize we already have this inconsistency 😁

@anseki
Copy link
Contributor Author

anseki commented Mar 16, 2017

Ok, I replaced those with const.

@jhnns jhnns merged commit ddf64e4 into webpack:master Mar 16, 2017
@jhnns
Copy link
Member

jhnns commented Mar 16, 2017

Published as 1.1.0

@anseki
Copy link
Contributor Author

anseki commented Mar 16, 2017

Thank you 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jhnns jhnns jhnns requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@anseki @jsf-clabot @jhnns