Refactor acls - Githubissues

spiral-project / daybed

Form validation and data storage API

http://daybed.rtfd.org/

BSD 3-Clause "New" or "Revised" License

53 stars 8 forks source link

Refactor acls #176

Closed Natim closed 9 years ago

Natim commented 9 years ago

[x] Removes Roles, Policy, Groups, Users
[x] Adds permissions and tokens
[x] Merge Backends.backend and backends.database
[x] Handle Everyone and Authenticated (for Anonymous and all Authenticated access)

Natim commented 9 years ago

Comments are not fold since you have commented the commit rather than the PR.

almet commented 9 years ago

yay, awesome!

leplatrem commented 9 years ago

Great that you could spend time hack on this !

Things look a LOT simpler at the first look :)

A few comments :

To be honest, I regret it was merged in a rush without documentation update
Was this reasonnably well tested ? Test cases look very light comparing to the amount of change
You test many sets intersections, using intermediary variables with explicit names would increase readibility IHMO
BTW I generally prefer the s1.intersection(s2) version for set intersections (matter of taste ?)

Anyway, good job and many thanks for the energy spent on this long awaited refactoring :)

Natim commented 9 years ago

We mostly removed so code but we didn't changed how the permission system works.

almet commented 9 years ago

@leplatrem yes, it was a long time needed refactoring. Goal is not to finish everything in one pull request but get things going on pretty smoothly. It's always time to change stuff that was committed.

Documentation will come along when we'll have something ready for prime time!

almet commented 9 years ago

Actually, +1 for the .intersecttion when dealing with sets. I liked the notation with & and ^ but we made some mistakes with them so it proves that explicit is better than implicit (is there a need to prove it still?)

Natim commented 9 years ago

Yes I will do that :)