<p></p>
<p dir="auto">I've created an issue with some UI-focussed review notes at <a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="2286023536" data-permission-text="Title is private" data-url="https://github.com/openstreetmap/openstreetmap-website/issues/4773" data-hovercard-type="issue" data-hovercard-url="/openstreetmap/openstreetmap-website/issues/4773/hovercard" href="https://github.com/openstreetmap/openstreetmap-website/issues/4773">#4773</a> .</p>
<p dir="auto">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</p>
<blockquote>
<p dir="auto">Great! Definitely more than one PR involved here 😄</p>
</blockquote>
<p dir="auto">and in March:</p>
<blockquote>
<p dir="auto">If you can find any way to move parts of this PR into separate PRs, then that would be great</p>
</blockquote>
<p dir="auto">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 <code class="notranslate">auth_button_preferred</code> 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.</p>
<p dir="auto">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 <a href="https://github.com/openstreetmap/openstreetmap-website/pull/4455#issuecomment-2007142676" data-hovercard-type="pull_request" data-hovercard-url="/openstreetmap/openstreetmap-website/pull/4455/hovercard">[1]</a>, <a href="https://github.com/openstreetmap/openstreetmap-website/pull/4455#issuecomment-2027017070" data-hovercard-type="pull_request" data-hovercard-url="/openstreetmap/openstreetmap-website/pull/4455/hovercard">[2]</a>. 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.</p>
<p dir="auto">So please, smaller, more focussed PRs next time! 😄 It will make things easier for everyone.</p>
<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />Reply to this email directly, <a href="https://github.com/openstreetmap/openstreetmap-website/pull/4455#issuecomment-2101033608">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/AAK2OLKIPSNUY22Q7GT7P23ZBJL3VAVCNFSM6AAAAABBLOL2OWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMBRGAZTGNRQHA">unsubscribe</a>.<br />You are receiving this because you are subscribed to this thread.<img src="https://github.com/notifications/beacon/AAK2OLO4GSYTQEZMSQH47DTZBJL3VA5CNFSM6AAAAABBLOL2OWWGG33NNVSW45C7OR4XAZNMJFZXG5LFINXW23LFNZ2KUY3PNVWWK3TUL5UWJTT5HM5IQ.gif" height="1" width="1" alt="" /><span style="color: transparent; font-size: 0; display: none; visibility: hidden; overflow: hidden; opacity: 0; width: 0; height: 0; max-width: 0; max-height: 0; mso-hide: all">Message ID: <span><openstreetmap/openstreetmap-website/pull/4455/c2101033608</span><span>@</span><span>github</span><span>.</span><span>com></span></span></p>
<script type="application/ld+json">[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "https://github.com/openstreetmap/openstreetmap-website/pull/4455#issuecomment-2101033608",
"url": "https://github.com/openstreetmap/openstreetmap-website/pull/4455#issuecomment-2101033608",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>