Skip to content

Date: Mark getSettings as experimental#10636

Merged
aduth merged 7 commits intomasterfrom
fix/10627
Oct 25, 2018
Merged

Date: Mark getSettings as experimental#10636
aduth merged 7 commits intomasterfrom
fix/10627

Conversation

@ajitbohra
Copy link
Copy Markdown
Member

@ajitbohra ajitbohra commented Oct 15, 2018

Fixes #10627

@ajitbohra ajitbohra added [Type] Breaking Change For PRs that introduce a change that will break existing functionality [Package] Date /packages/date labels Oct 15, 2018
@ajitbohra ajitbohra requested review from Soean and aduth October 15, 2018 21:21
Soean
Soean requested changes Oct 15, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@Soean Soean left a comment •
edited
Loading

Choose a reason for hiding this comment

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

If we deprecate getSettings(), we should change all parts where we import and use it.

aduth
aduth reviewed Oct 15, 2018
View reviewed changes
@gziolo gziolo added this to the 4.2 - API freeze milestone Oct 16, 2018
@ajitbohra
Copy link
Copy Markdown
Member Author

ajitbohra commented Oct 17, 2018 •
edited
Loading

@Soean ahh totally missed that deprecating for the first time 😅

@aduth added the hint to discourage the usage. If I am not wrong we need to update the changelog and bump the version for the date package, just noticed changelog the last version is 2.1.0 and package.json version is 2.0.2!

aduth
aduth reviewed Oct 23, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@aduth aduth left a comment

Choose a reason for hiding this comment

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

Code-wise this looks good, but it missed the 4.1.0 release so we'll need to bump back the deprecated version.

@ajitbohra
Copy link
Copy Markdown
Member Author

ajitbohra commented Oct 24, 2018

@aduth

version bumped

need help on this

changelog the last version is 2.1.0 and package.json version is 2.0.2!

@aduth
Copy link
Copy Markdown
Member

aduth commented Oct 24, 2018

@ajitbohra Ah, sorry for missing the question. We never need to touch the package.json in any pull request, and it's expected the version would lag behind if there are "Unreleased" changes. We should include the note in CHANGELOG.md for the deprecation though, as a minor release.

See: https://github.com/WordPress/gutenberg/blob/master/CONTRIBUTING.md#maintaining-changelogs

@ajitbohra
Copy link
Copy Markdown
Member Author

ajitbohra commented Oct 24, 2018

Note added to change log

Soean
Soean reviewed Oct 24, 2018
View reviewed changes
aduth
aduth reviewed Oct 24, 2018
View reviewed changes
@aduth @ajitbohra
Remove deprecated from 4.2
945ce24 
Co-Authored-By: ajitbohra <ajit@lubus.in>
Soean
Soean approved these changes Oct 24, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@Soean Soean left a comment

Choose a reason for hiding this comment

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

LGTM

Soean
Soean reviewed Oct 25, 2018
View reviewed changes
@Soean @ajitbohra
Marks as unreleased
2e7638e 
Co-Authored-By: ajitbohra <ajit@lubus.in>
aduth
aduth approved these changes Oct 25, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@aduth aduth left a comment

Choose a reason for hiding this comment

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

In some final testing I came across fact that we export moment as well, which (for different reasons than this) is probably something we don't want to expose on the API. Thoughts?

Not blocking for this pull request.

@aduth aduth merged commit 830c9ec into master Oct 25, 2018
@aduth aduth deleted the fix/10627 branch October 25, 2018 14:51
@youknowriad
Copy link
Copy Markdown
Contributor

youknowriad commented Oct 25, 2018

In some final testing I came across fact that we export moment as well, which (for different reasons than this) is probably something we don't want to expose on the API. Thoughts?

We should definitely set "moment" as experimental (or get rid of it) as well. This is something I wanted to do but not sure if I'll have the time to do it.

@ajitbohra
Copy link
Copy Markdown
Member Author

ajitbohra commented Oct 25, 2018

Yes, I remember the previous discussion around not exposing moment. Will have a look and work on the PR for the same.

antpb pushed a commit to antpb/gutenberg that referenced this pull request Oct 26, 2018
@ajitbohra @antpb
Date: Mark getSettings as experimental (WordPress#10636)
667d165 
* Date: Mark getSettings as experimental

* Update all references to __experimentalGetSettings

* Bump deprecated version

* Add change to changelog

* Remove deprecated from 4.2

Co-Authored-By: ajitbohra <ajit@lubus.in>

* Marks as unreleased

Co-Authored-By: ajitbohra <ajit@lubus.in>
daniloercoli added a commit that referenced this pull request Oct 26, 2018
@daniloercoli
Merge branch 'master' of https://github.com/WordPress/gutenberg into …
7659051 
…rnmobile/merge-blocks-on-backspace

* 'master' of https://github.com/WordPress/gutenberg:
  Do not add isDirty prop to DOM (#11093)
  Format API (#10209)
  Remove 4.2 deprecated features (#10952)
  Update `@wordpress/hooks` README to include namespace mention (#11061)
  Feature: save lock control via actions (#10649)
  Fix usage of `preg_quote()` (#10998)
  Update plugin version to 4.1.1 (#11078)
  Improve preloading request code (#11007)
  Fix dynamic blocks not rendering in the frontend (#11050)
  Media & Text: Fixing vertical alignment of the image (#11025)
  Date: Mark getSettings as experimental (#10636)
  Improve handling of centered 1-col galleries with small images (#11040)
  Use better help text for ALT text input; fixes #8391. (#11052)

# Conflicts:
#	packages/editor/src/components/rich-text/index.native.js
@jeffstieler jeffstieler mentioned this pull request Dec 7, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Soean Soean Soean approved these changes

@aduth aduth aduth approved these changes

Assignees

No one assigned

Labels

[Package] Date /packages/date [Type] Breaking Change For PRs that introduce a change that will break existing functionality

Projects

None yet

Milestone

4.2

Development

Successfully merging this pull request may close these issues.

5 participants

@ajitbohra @aduth @youknowriad @Soean @gziolo