Skip to content
This repository has been archived by the owner on Nov 5, 2019. It is now read-only.

Switching flask_util and django_util to use DictionaryStorage instead… #383

Merged

Conversation

theacodes
Copy link
Contributor

… of custom classes.

def locked_delete(self):
if _CREDENTIALS_KEY in self.session:
del self.session[_CREDENTIALS_KEY]
return DictionaryStorage(request.session, key=_CREDENTIALS_KEY)

This comment was marked as spam.

@dhermes
Copy link
Contributor

dhermes commented Jan 11, 2016

LGTM. Coverage drop is fine; math and stuff.

@nathanielmanistaatgoogle
Copy link
Contributor

Please rewrite your commit message to conform to http://chris.beams.io/posts/git-commit/#seven-rules?

Change content looks fine.

@theacodes
Copy link
Contributor Author

@nathanielmanistaatgoogle was your primary issue the tense of the verb? I've fixed that. If you want more detail in the commit message I can add that as well.

@nathanielmanistaatgoogle
Copy link
Contributor

"Limit the subject line to 50 characters" is the one I usually notice, and that I noticed here. Notice how the title of the pull request overflows and is truncated with an ellipsis.

@theacodes
Copy link
Contributor Author

Hmm. I think that errs on the side of rules for the sake of rules, but I shortened it a bit (to 60 characters).

@nathanielmanistaatgoogle
Copy link
Contributor

Yeah, I don't like those rules either, but they're where the industry is going and I do favor consistency for metadata like commit messages.

@theacodes
Copy link
Contributor Author

Yeah, I don't like those rules either, but they're where the industry is going and I do favor consistency for metadata like commit messages.

Agreed, but sometimes 50 characters isn't enough. :)

Let me know if there's anything else I need to do here. Travis should turn green, the code itself hasn't changed since the PR was opened.

@nathanielmanistaatgoogle
Copy link
Contributor

This is fine. Will merge when tests repass.

@nathanielmanistaatgoogle
Copy link
Contributor

I don't know what's up with Travis. Merging.

nathanielmanistaatgoogle added a commit that referenced this pull request Jan 11, 2016
Switched flask_util and django_util to use DictionaryStorage.
@nathanielmanistaatgoogle nathanielmanistaatgoogle merged commit 8f8df1f into googleapis:master Jan 11, 2016
@theacodes
Copy link
Contributor Author

Thanks!

@dhermes dhermes mentioned this pull request Jan 29, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants