[openstreetmap/openstreetmap-website] Disallow username changes to user_n if n isn't their id (PR #4218)
Andy Allan
notifications at github.com
Thu Jan 18 15:15:42 UTC 2024
> I guess maybe we should just disable them fully - probably the idea of the auto-generated todo putting a numeric limit in instead was to stop people making them worse but obviously we don't do that.
Well maybe in future I should refuse any PRs that increase the number! But I think other people would be unhappy with that approach.
I don't want to turn it off, since it's making a point that needs to be made. `user.rb` is far too big to understand at a glance, and imho should be refactored, but like you say that's a longer-term TODO and I can't block a security fix by waiting for the refactoring.
I suspect what we should do is make it a per-file exemption rather than a global one, so that when we refactor `user.rb` we don't find that there are many other files that have grown in the meantime.
> Previously pull requests were merged with a separate commit changing rubocop config: #4284 [393914d](https://github.com/openstreetmap/openstreetmap-website/commit/393914d35b970e98c825dc650f679fcb12b6655e).
:shrug: So I'm inconsistent, perhaps? But in general I like to make sure that tests pass for all commits, so that `git bisect` works smoothly. And it was easy for me to fix while I was making commits on this PR anyway. But perfect is the enemy of good so it's not a super strict rule or anything.
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/4218#issuecomment-1898673879
You are receiving this because you are subscribed to this thread.
Message ID: <openstreetmap/openstreetmap-website/pull/4218/c1898673879 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20240118/59b73395/attachment-0001.htm>
More information about the rails-dev
mailing list