[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