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

[FEAT] remove all stale feature flags #5384

Merged
merged 2 commits into from Mar 26, 2018

Conversation

Projects
None yet
6 participants
@runspired
Copy link
Contributor

runspired commented Mar 16, 2018

As discussed in the face-to-face, our feature flags have gone very stale and we
we decided to go a different direction on each. This is a debt-collection PR to remove
our stale feature flags and reset the feature-flag clock.

All feature flags that were previously null were removed with their off branch left behind.
All feature flags that were previously true were removed with their on branch left behind.

runspired added some commits Mar 16, 2018

@runspired

This comment has been minimized.

Copy link
Contributor

runspired commented Mar 16, 2018

Starting a list of tickets to update when this merges:

@jonnii

This comment has been minimized.

Copy link

jonnii commented Mar 16, 2018

@runspired sad about ds-pushpayload-return, it's pretty useful for people who use websockets =(

@runspired

This comment has been minimized.

Copy link
Contributor

runspired commented Mar 16, 2018

@jonnii are you saying you are using that feature!?

Are you aware of store.push ?

@jonnii

This comment has been minimized.

Copy link

jonnii commented Mar 16, 2018

@runspired I didn't realize push returned the record it created, you can ignore my comment!

@urbany

This comment has been minimized.

Copy link

urbany commented Mar 17, 2018

Related: #5265

@sly7-7

This comment has been minimized.

Copy link
Contributor

sly7-7 commented Mar 19, 2018

@runspired All green, so I guess this is good to merge ?

Concerning the push-payload thing, store.push does not use the serializer, so if you have an other format coming from the server, you can't directly use store.push. I would to use that feature, but as it is going to be removed, I can do without it (keeping the ids will push, calling pushPayload, then peekRecord on each id). Seems brittle, but at least that only use public api.

@rwjblue
Copy link
Member

rwjblue left a comment

LGTM...

@runspired

This comment has been minimized.

Copy link
Contributor

runspired commented Mar 19, 2018

@sly7-7 an example websocket push that is much less brittle than even the pushPayload use today

// ... payload is raw payload received
let store = this.get('store');
let ModelClass = store.modelFor('foo');
let serializer = store.serializerFor('foo');
let normalized = store.normalizeResponse(store, ModelClass, payload, null, 'query');

return store.push(normalized);

@bmac bmac merged commit ea6aadd into emberjs:master Mar 26, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment