<p></p>
<blockquote>
<blockquote>
<p dir="auto"><a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/tomhughes/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://github.com/tomhughes">@tomhughes</a> <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/gravitystorm/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://github.com/gravitystorm">@gravitystorm</a> any more changes required for this PR?</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. For example, you fixed the blocked page (headings and paragraphs) in the latest commit, but that could be a completely separate PR that can be reviewed separately and merged first. Are there other things like that here?</p>
<p dir="auto">Perhaps the "login" vs "log in" text changes? The logo change? Or other similar things that can be treated separately? Large pull requests are quadratically hard to deal with - not only is there more to review, but each time I come to re-review the PR, more things have changed, and I need to read and re-read both the changed and also the unchanged parts over and over. So it's really worthwhile to break down the PR into smaller parts, that will ideally only need one maintainer to review and merge.</p>
</blockquote>
<p dir="auto">I have created 3 separate PRs as you suggested:</p>
<ul dir="auto">
<li><a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="2064057773" data-permission-text="Title is private" data-url="https://github.com/openstreetmap/openstreetmap-website/issues/4455" data-hovercard-type="pull_request" data-hovercard-url="/openstreetmap/openstreetmap-website/pull/4455/hovercard" href="https://github.com/openstreetmap/openstreetmap-website/pull/4455">#4455</a></li>
<li><a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="2186820356" data-permission-text="Title is private" data-url="https://github.com/openstreetmap/openstreetmap-website/issues/4578" data-hovercard-type="pull_request" data-hovercard-url="/openstreetmap/openstreetmap-website/pull/4578/hovercard" href="https://github.com/openstreetmap/openstreetmap-website/pull/4578">#4578</a></li>
<li><a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="2188496320" data-permission-text="Title is private" data-url="https://github.com/openstreetmap/openstreetmap-website/issues/4580" data-hovercard-type="pull_request" data-hovercard-url="/openstreetmap/openstreetmap-website/pull/4580/hovercard" href="https://github.com/openstreetmap/openstreetmap-website/pull/4580">#4580</a></li>
</ul>
<p dir="auto">I understand the frustration with large PRs like this. They are not pleasant neither for reviewers nor the contributor.</p>
<p dir="auto">The moving target for contributor (master branch being updated resulting in merge conflicts) triggers me to rebase the PR branch, breaking the correlation between comments and code, and that makes reviewing the PR even more unpleasant, usually resulting in full re-review.</p>
<p dir="auto">In case of this PR there is a really big modification that cannot be reasonably split into smaller PRs, namely "joining sign-up and terms screens into one" which touches large portions of <code class="notranslate">users_controller.rb</code> and related views.</p>
<p dir="auto">However, it seems that the small PRs I created above will not reduce the size of this PR significantly, since this PR would modify or move the same pieces of code. I will have to wait until they are merged before rebasing this PR once again...</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-1999788970">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/AAK2OLLUTK2K6XAGRCA5ZG3YYMAVTAVCNFSM6AAAAABBLOL2OWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOJZG44DQOJXGA">unsubscribe</a>.<br />You are receiving this because you are subscribed to this thread.<img src="https://github.com/notifications/beacon/AAK2OLJ5H2YKOFNJULZAPRLYYMAVTA5CNFSM6AAAAABBLOL2OWWGG33NNVSW45C7OR4XAZNMJFZXG5LFINXW23LFNZ2KUY3PNVWWK3TUL5UWJTTXGJN2U.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/c1999788970</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-1999788970",
"url": "https://github.com/openstreetmap/openstreetmap-website/pull/4455#issuecomment-1999788970",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>