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

Remove "show_unanchored_annotations" feature #2741

Merged
merged 1 commit into from Nov 26, 2015

Conversation

Projects
None yet
3 participants
@nickstenning
Contributor

nickstenning commented Nov 25, 2015

We've discussed this feature and decided that it can't be turned on in its current state. This commit removes it.

Note that the feature flag in h/features.py remains until these changes are deployed out to production.

Remove "show_unanchored_annotations" feature
We've discussed this feature and decided that it can't be turned on in
its current state. This commit removes it.

Note that the feature flag in `h/features.py` remains until these
changes are deployed out to production.
@dwhly

This comment has been minimized.

Show comment
Hide comment
@dwhly

dwhly Nov 25, 2015

Member

Nick is there any reason staff can't continue to use it?


Dan / +1 650 274 7647 mobile

On Wed, Nov 25, 2015 at 11:46 AM -0800, "Nick Stenning" notifications@github.com wrote:

We've discussed this feature and decided that it can't be turned on in its current state. This commit removes it.

Note that the feature flag in h/features.py remains until these changes are deployed out to production.

You can view, comment on, or merge this pull request online at:

  #2741

Commit Summary

Remove "show_unanchored_annotations" feature

File Changes

M
h/static/scripts/directive/test/thread-test.coffee
(4)


M
h/static/scripts/directive/thread.coffee
(4)


M
h/static/styles/threads.scss
(8)


M
h/templates/client/thread.html
(5)

Patch Links:

https://github.com/hypothesis/h/pull/2741.patch
https://github.com/hypothesis/h/pull/2741.diff


Reply to this email directly or view it on GitHub.

Member

dwhly commented Nov 25, 2015

Nick is there any reason staff can't continue to use it?


Dan / +1 650 274 7647 mobile

On Wed, Nov 25, 2015 at 11:46 AM -0800, "Nick Stenning" notifications@github.com wrote:

We've discussed this feature and decided that it can't be turned on in its current state. This commit removes it.

Note that the feature flag in h/features.py remains until these changes are deployed out to production.

You can view, comment on, or merge this pull request online at:

  #2741

Commit Summary

Remove "show_unanchored_annotations" feature

File Changes

M
h/static/scripts/directive/test/thread-test.coffee
(4)


M
h/static/scripts/directive/thread.coffee
(4)


M
h/static/styles/threads.scss
(8)


M
h/templates/client/thread.html
(5)

Patch Links:

https://github.com/hypothesis/h/pull/2741.patch
https://github.com/hypothesis/h/pull/2741.diff


Reply to this email directly or view it on GitHub.

@nickstenning

This comment has been minimized.

Show comment
Hide comment
@nickstenning

nickstenning Nov 25, 2015

Contributor

Nick is there any reason staff can't continue to use it?

If we have no intention of releasing the feature to users, I'd rather we didn't keep it.

Feature flags are intended to be used as a release mechanism, not a way of having different sets of features for different groups of users, so I'd rather we didn't use them that way.

Contributor

nickstenning commented Nov 25, 2015

Nick is there any reason staff can't continue to use it?

If we have no intention of releasing the feature to users, I'd rather we didn't keep it.

Feature flags are intended to be used as a release mechanism, not a way of having different sets of features for different groups of users, so I'd rather we didn't use them that way.

@dwhly

This comment has been minimized.

Show comment
Hide comment
@dwhly

dwhly Nov 25, 2015

Member

Just as a bridge to when we are able to create a permanent home, which hopefully will be soon. Clearly not a long term solution.


Dan / +1 650 274 7647 mobile

On Wed, Nov 25, 2015 at 12:20 PM -0800, "Nick Stenning" notifications@github.com wrote:

Nick is there any reason staff can't continue to use it?

If we have no intention of releasing the feature to users, I'd rather we didn't keep it.

Feature flags are intended to be used as a release mechanism, not a way of having different sets of features for different groups of users, so I'd rather we didn't use them that way.


Reply to this email directly or view it on GitHub.

Member

dwhly commented Nov 25, 2015

Just as a bridge to when we are able to create a permanent home, which hopefully will be soon. Clearly not a long term solution.


Dan / +1 650 274 7647 mobile

On Wed, Nov 25, 2015 at 12:20 PM -0800, "Nick Stenning" notifications@github.com wrote:

Nick is there any reason staff can't continue to use it?

If we have no intention of releasing the feature to users, I'd rather we didn't keep it.

Feature flags are intended to be used as a release mechanism, not a way of having different sets of features for different groups of users, so I'd rather we didn't use them that way.


Reply to this email directly or view it on GitHub.

@nickstenning

This comment has been minimized.

Show comment
Hide comment
@nickstenning

nickstenning Nov 25, 2015

Contributor

I don't see any justification for giving staff one experience and users another. If it's not good enough for users, it's not good enough for staff. If it's good enough for staff, it's good enough for users.

Contributor

nickstenning commented Nov 25, 2015

I don't see any justification for giving staff one experience and users another. If it's not good enough for users, it's not good enough for staff. If it's good enough for staff, it's good enough for users.

@robertknight

This comment has been minimized.

Show comment
Hide comment
@robertknight

robertknight Nov 25, 2015

Contributor

If it's good enough for staff, it's good enough for users.

I agree with that line of thought but I do find the current implementation useful and, from a completely naive perspective of someone who hasn't seen internal discussions about it, I wondered why we haven't shipped what we have.

Contributor

robertknight commented Nov 25, 2015

If it's good enough for staff, it's good enough for users.

I agree with that line of thought but I do find the current implementation useful and, from a completely naive perspective of someone who hasn't seen internal discussions about it, I wondered why we haven't shipped what we have.

@nickstenning

This comment has been minimized.

Show comment
Hide comment
@nickstenning

nickstenning Nov 26, 2015

Contributor

Just in case it's not clear, this isn't me deciding unilaterally to remove this. I discussed it with @judell, who isn't happy shipping what we have.

Contributor

nickstenning commented Nov 26, 2015

Just in case it's not clear, this isn't me deciding unilaterally to remove this. I discussed it with @judell, who isn't happy shipping what we have.

robertknight added a commit that referenced this pull request Nov 26, 2015

Merge pull request #2741 from hypothesis/remove-show-unanchored-feature
Remove "show_unanchored_annotations" feature

@robertknight robertknight merged commit fd96598 into master Nov 26, 2015

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@nickstenning nickstenning deleted the remove-show-unanchored-feature branch Nov 26, 2015

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