Skip to content
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

CookieLaw and enhanced privacy settings #54

Merged
merged 30 commits into from
Jul 10, 2015
Merged

CookieLaw and enhanced privacy settings #54

merged 30 commits into from
Jul 10, 2015

Conversation

keul
Copy link
Member

@keul keul commented Jul 6, 2015

See #51

@@ -6,6 +6,12 @@ There's a frood who really knows where his towel is.
2.3 (unreleased)
^^^^^^^^^^^^^^^^

- Flake8 cleanup
Copy link
Member

Choose a reason for hiding this comment

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

IMO, changelog entries should provide useful information for end users, developers and integrators; Flake8 fixes doesn't fall under this... but that's only MO.

Copy link
Member Author

Choose a reason for hiding this comment

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

@hvelarde no opposition about this, is not really useful.
It take me one minute for remove the entry



def add_privacy_setting(context):
''' Apply upgrade profile '''
Copy link
Member

Choose a reason for hiding this comment

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

triple double quotes in docstrings, please

@hvelarde
Copy link
Member

hvelarde commented Jul 6, 2015

@keul nice work! the only thing I would love to change in the name and description of the field... I think privacy is not a good one, neither "Severe privacy".

In the future we probably want to split this feature out of this package.

also I've seen this implemented in some European news sites with a small enhancement: showing a small dialog when you visit their sites telling you that they are using cookies; we could do the same later.

selection_001

@keul
Copy link
Member Author

keul commented Jul 6, 2015

@hvelarde if you can find a better name for the field I can change it! I simply didn't find any better option.

About the global warning: we use it too, but in another package (because "social" is not the only possible cookie origin) but this is a different (and complex) argument as every EU country interpreted the law in a different way.

@keul
Copy link
Member Author

keul commented Jul 8, 2015

Still left:

  • change urllib.quote with urlencode (this needs some changes to the code)
  • find a better name for the privacy field (suggestion welcome)

xmlns:gs="http://namespaces.zope.org/genericsetup"
i18n_domain="sc.social.like">

<gs:registerProfile
Copy link
Member

Choose a reason for hiding this comment

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

I think we also need to hide this profile

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep! Right

@keul
Copy link
Member Author

keul commented Jul 8, 2015

@hvelarde hidden

@@ -72,6 +72,19 @@ class IProvidersSchema(Interface):
vocabulary=styles,
)

privacy = schema.Bool(
Copy link
Member

Choose a reason for hiding this comment

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

what about avoid_tracking of do_not_track?

@agnogueira @rodfersou comments?

@keul
Copy link
Member Author

keul commented Jul 10, 2015

@hvelarde thanks for contributing, give me some days to check those changes.
IMHO "Do not track" is a good label too, 👍

@keul
Copy link
Member Author

keul commented Jul 10, 2015

@hvelarde I used the do_not_track version (hoping this will be the final choice)
Also: new URL schema changes are OK.

hvelarde added a commit that referenced this pull request Jul 10, 2015
CookieLaw and enhanced privacy settings
@hvelarde hvelarde merged commit 2f26607 into master Jul 10, 2015
@hvelarde hvelarde deleted the cookie-law branch July 10, 2015 21:51
@hvelarde
Copy link
Member

@keul awesome! could you please update the changelog on the master branch to reflect better what was done? don't hesitate on adding a whole paragraph.

after that we can make a release if you need it.

@keul
Copy link
Member Author

keul commented Jul 13, 2015

@hvelarde I added few words in the CHANGES file, just because I already added a new whole section in the main README. Feel free to extend/fix it!

If you can spread a new release, it will be great.

@hvelarde
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants