Api more security #1196
Api more security #1196
Conversation
|
@pliablepixels Can I suggest that since github tracks all changes, blame, etc and we do not have any precedent of using comments to attribute a change that we remove the "//PP" comments? |
|
@SteveGilvarry - yes, this was actually a way for me to track local changes before committing - but its not needed - I'll strip and push again |
… the day! Gone, and a cloud in my heart. - Tennyson
|
Maybe the #1155 ones can go altogether also |
|
Everything in #1155 is also in this (including bug fixes to it) - which is why I closed 1155 |
|
oh you mean PP? Yes, in the update I pushed above with a heart wrenching quote from Lord Tennyson, PP is gone from there too. There will be other fragments of past PRs that will still have PP - I'll get around to removing them later, after I consult with my legacy planner on what this means to my legacy. |
| return; | ||
| } | ||
| // #1155 fixes |
There was a problem hiding this comment.
@pliablepixels Sorry I meant these "// #1155 fixes" ones, just the comment as again we can always see what belongs to what in Github. Or unless you think something needs to say why it exists in which case maybe change the comment to something more meaningful.
There was a problem hiding this comment.
Oh ok - yes, what I really should do is refer to issues in commit messages - like I do in zmN - its a good way to map code to issues later. But I don't think I can find any quote from tennyson that will be apt for this change
There was a problem hiding this comment.
Actually @SteveGilvarry - I just remembered, its an easy way for me to say "This change went in due to user role auth checking" - should I remove it?
There was a problem hiding this comment.
Something like //User role check if you think it would help someone reading the code understand it's purpose, but for me doesn't need to talk about a change. That is all covered by commits and PR in github.
|
done |
Addresses issue #1195