Skip to content
This repository has been archived by the owner on Jul 9, 2018. It is now read-only.

Url Module: Drop the browser built file and the useless dependencies #23

Merged
merged 2 commits into from
Aug 1, 2017

Conversation

youknowriad
Copy link
Collaborator

I tested using the url module in Gutenberg using npm ln.

The good news is:

  • The "module" (ES6) build is working as expected
  • The "main" (ES5) build is working as expected

The bad news is:

  • The browser build is broken. I decided to remove it in this PR, we could introduce it later when we decide how to export the modules (global variable for example)

I also removed the url dependencies in this PR because these are included in Node.js by default core and webpack automatically include them if they're used in a webpack built application.

I'm hoping we could release the first version of this module after the merge. I hesitated to update the package version to 1.0.0 (any thoughts on this?).

@youknowriad youknowriad self-assigned this Jul 31, 2017
@codecov
Copy link

codecov bot commented Jul 31, 2017

Codecov Report

Merging #23 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #23   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           1      1           
  Lines           4      4           
=====================================
  Hits            4      4

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d60f97c...7981a08. Read the comment docs.

@aduth
Copy link
Member

aduth commented Jul 31, 2017

I'm hoping we could release the first version of this module after the merge. I hesitated to update the package version to 1.0.0 (any thoughts on this?).

I don't mind a pre-1.0 release, but we should still follow SemVer and prefer incrementing on the minor line, not the patch line (unless a release is in-fact a patch release).

@youknowriad
Copy link
Collaborator Author

I don't mind a pre-1.0 release,

👍 Updated the version to 1.0.0-rc.1

@aduth
Copy link
Member

aduth commented Jul 31, 2017

Sorry, by pre-1.0, I specifically meant a 0.1.0. Release candidate implies that the release includes all features we'd want for a 1.0.0 release, which... may or may not be the case. For this module, it's pretty sparse. A 1.0.0 with addQueryArgs only seems maybe premature. Then again, adding new features at minor release is fair enough (new methods in 1.1.0, 1.2.0, etc).

Do you think this is something we ought to discuss at tomorrow's JavaScript meeting?

@youknowriad
Copy link
Collaborator Author

@aduth yes, let's discuss this in tomorrow's meeting. I don't have any opinion on the subject, I barely pay attention to the version number of the modules I use :)

@youknowriad
Copy link
Collaborator Author

As discussed in the meeting, I updated the version to 0.1.0-beta.1. Merging.

@youknowriad youknowriad merged commit 1e35b26 into master Aug 1, 2017
@youknowriad youknowriad deleted the fix/url-module branch August 1, 2017 14:10
@aduth aduth mentioned this pull request Aug 2, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants