[openstreetmap/openstreetmap-website] Re-arrange login and signup screens as discussed in #4128 (PR #4455)

Andy Allan notifications at github.com
Wed May 8 17:09:14 UTC 2024


I've created an issue with some UI-focussed review notes at #4773 .

I just want to also touch on reviewing this PR in general. I think in future I would push back harder on doing so many things at once, all in the same PR. I wasn't joking back in August 2023 when I said

> Great! Definitely more than one PR involved here 😄

and in March:

> If you can find any way to move parts of this PR into separate PRs, then that would be great

I appreciate that some small parts were separated off, but there was still a large amount of both functionality changes happening at the same time as significant user interface redesign going on. There was no need, for example, to have the `auth_button_preferred` helper in the same PR as removing the contributor terms, in the same PR as fixing the 3rd-party signup workflow, in the same PR as discussing auth round trip security, etc etc. 

In particular, although there were several conversations about how hard this was making it for us to review the PR, the conversation then moved on to pressuring Tom and I to review and merge the PR regardless [[1]](https://github.com/openstreetmap/openstreetmap-website/pull/4455#issuecomment-2007142676), [[2]](https://github.com/openstreetmap/openstreetmap-website/pull/4455#issuecomment-2027017070). I have to be honest that it left a bad taste in my mouth, and I'm keen to avoid that happening again in future.

So please, smaller, more focussed PRs next time! :smile: It will make things easier for everyone.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/4455#issuecomment-2101033608
You are receiving this because you are subscribed to this thread.

Message ID: <openstreetmap/openstreetmap-website/pull/4455/c2101033608 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20240508/c3ff8a4c/attachment.htm>


More information about the rails-dev mailing list