bulbs-poll - Githubissues

theonion / django-bulbs

DEPRECATED: This project is now part of the Mono Repo (https://github.com/theonion/omni)

MIT License

26 stars 7 forks source link

bulbs-poll #96

Closed collin closed 8 years ago

collin commented 8 years ago

@mparent @benghaziboy @MichaelButkovic @kand

Added notes for running django-bulbs against the onion-services elasticsearch.

Is this clear enough?

TODO

[x] move things like 'Intentionally blank' into constants
[x] setup linter Ran linter locally on new poll/ files and tests
[x] hoist SodaheadAPIError out of Poll namespace
[x] hoist sodahead_id generation into function
[x] use if not self.sodahead_id
[x] favor == over is for equality checking (vs identity checking)
- [x] some string comparisons
- [x] some integer comparisons (http status codes)
[x] write comment explaining rational behind sodahead integration
[ ] re-open bengazi investigation to get to the truth
[x] test serializer against explicit string values
[ ] class level @mock_vault(SECRETS) @mparent61 some python 2/3 issues around copying the mock_vault implementation over I'm going to hold off on that for now.

collin commented 8 years ago

@benghaziboy @mparent61 This has changed a bit since the last request for comments. Can we get another look?

camsom commented 8 years ago

I want to know what really happened at that compound

mparent61 commented 8 years ago

Spoke with @MelizzaP, going to rollout v0.7.0 (breaking Special Coverage changes) that requires site updates applied today + Monday. This means this PR can't be merged + rolled out to sites until roughly Tue (2015-02-16).

https://github.com/theonion/django-bulbs/pull/110

mparent61 commented 8 years ago

Also need to coordinate with @benghaziboy re: some potential issues with 0.6.66 (Contribution celery tasks) we're investigating.

Bottom line: Need to coordinate before this is pushed out to sites.

collin commented 8 years ago

@mparent61 @benghaziboy @MichaelButkovic

Alright, I've addressed most of the notes and some of the extra ones you had on the PR @MelizzaP started for polls phase ||.

Any last notes?

camsom commented 8 years ago

we need a way to silence all these fixtures in PRs, but other than that.... :+1:

collin commented 8 years ago

@mparent61 :+1: on coordination /cc @spra85

MichaelButkovic commented 8 years ago

LGTM

MichaelButkovic commented 8 years ago

actually no lgtm until re-open bengazi investigation to get to the truth is finished

mparent61 commented 8 years ago

Spoke with @collin, going to review this THUR AM after merging with @MelizzaP's branch and related PR comments

mparent61 commented 8 years ago

One more nit: I'd move the VCR files into a vcr-themed subdirectory ("vcr", "cassettes", etc) to clearly separate them from similarly-named test_*.py test files. This just requires a simple tweak to the make_vcr helper function and will benefit future VCR users too.

mparent61 commented 8 years ago

Other than my comments, :+1:

collin commented 8 years ago

I got the poll cassettes moved to test/poll/test_data/cassettes

MichaelButkovic commented 8 years ago

:+1:

collin commented 8 years ago

Alright @mparent61 I think we have this all covered.

What's our rollout pipeline looking like?

mparent61 commented 8 years ago

One Python 3 test is still failing: https://travis-ci.org/theonion/django-bulbs/jobs/113500999