in giving others confidence in the code you've written, and can
greatly speed up the process of merging in new code.
-When hacking, you should:
+When contributing, you should:
* Write new tests to cover the new functionality you've added.
* Where appropriate, modify existing tests to reflect new or changed
developers to read the code and satisfy themselves that it's doing the
right thing.
-When hacking, you should:
+When contributing, you should:
-* Comment your code - don't go overboard, but explain the bits which
+* Comment your code where necessary - explain the bits which
might be difficult to understand what the code does, why it does it
and why it should be the way it is.
* Check existing comments to ensure that they are not misleading.
## Committing
-When you submit patches, the project maintainer has to read them and
+When you submit your changes, the project maintainers have to read them and
understand them. This is difficult enough at the best of times, and
-misunderstanding patches can lead to them being more difficult to
-merge. To help with this, when submitting you should:
+misunderstanding commits can lead to them being more difficult to
+merge. To help with this, when committing you should:
-* Split up large patches into smaller units of functionality.
+* Split up large commits into smaller units of functionality.
* Keep your commit messages relevant to the changes in each individual
-unit.
+commit.
When writing commit messages please try and stick to the same style as
other commits, namely:
For simple commits the one line summary is often enough and the body
of the commit message can be left out.
-## Sending the patches
+## Pull Requests
If you have forked on GitHub then the best way to submit your patches is to
push your changes back to GitHub and then send a "pull request" on GitHub.
-Otherwise you should either push your changes to a publicly visible git repository
-and send the details to the [rails-dev](https://lists.openstreetmap.org/listinfo/rails-dev)
-list or generate patches with `git format-patch` and send them to the
-[rails-dev](https://lists.openstreetmap.org/listinfo/rails-dev) list.
+If your pull request is small, for example one or two commits each containing
+only a few lines of code, then it is easy for the maintainers to review.
+
+If you are creating a larger pull request, then please help the maintainers
+with making the reviews as straightforward as possible:
+
+* The smaller the PR, the easier it is to review. In particular if a PR is too
+ large to review in one sitting, or if changes are requested, then the
+ maintainer needs to repeatedly re-read code that has already been considered.
+* The commit history is important. This is a large codebase, developed over many
+ years by many developers. We frequently need to read the commit history (e.g.
+ using `git blame`) to figure out what is going on. So small, understandable,
+ and relevant commits are important for other developers looking back at your
+ work in future.
+
+If you are creating a large pull request then please:
+
+* Consider splitting your pull request into multiple PRs. If part of your work
+ can be considered standalone, or is a foundation for the rest of your work,
+ please submit it separately first.
+* Avoid including "fixup" commits. If you have added a fixup commit (for example
+ to fix a rubocop warning, or because you changed your own new code) please
+ combine the fixup commit into the commit that introduced the problem.
+ `git rebase -i` is very useful for this.
+* Avoid including "merge" commits. If your PR can no longer be merged cleanly
+ (for example, an unrelated change to Gemfile.lock on master now conflicts with
+ your PR) then please rebase your PR onto the latest master. This allows you to
+ fix the conflicts, while keeping the PR a straightforward list of commits. If
+ there are no conflicts, then there is no need to rebase anything.
$("#changeset_" + id).find("a.changeset_id").simulate("click", e);
}
+ function displayFirstChangesets(html) {
+ $("#sidebar_content .changesets").html(html);
+ }
+
+ function displayMoreChangesets(html) {
+ $("#sidebar_content .changeset_more").replaceWith(html);
+ var oldList = $("#sidebar_content .changesets ol").first();
+ var newList = oldList.next("ol");
+ newList.children().appendTo(oldList);
+ newList.remove();
+ }
+
function update() {
var data = { list: "1" };
method: "GET",
data: data,
success: function (html) {
- $("#sidebar_content .changesets").html(html);
+ displayFirstChangesets(html);
updateMap();
}
});
$(this).hide();
div.find(".loader").show();
- $.get($(this).attr("href"), function (data) {
- div.replaceWith(data);
+ $.get($(this).attr("href"), function (html) {
+ displayMoreChangesets(html);
updateMap();
});
}
require "application_system_test_case"
class HistoryTest < ApplicationSystemTestCase
+ PAGE_SIZE = 20
+
test "atom link on user's history is not modified" do
user = create(:user)
create(:changeset, :user => user, :num_changes => 1) do |changeset|
assert_css "link[type='application/atom+xml'][href$='#{user_path(user)}/history/feed']", :visible => false
end
+
+ test "have only one list element on user's changesets page" do
+ user = create(:user)
+ create_visible_changeset(user, "first-changeset-in-history")
+ create_visible_changeset(user, "bottom-changeset-in-batch-2")
+ (PAGE_SIZE - 1).times do
+ create_visible_changeset(user, "next-changeset")
+ end
+ create_visible_changeset(user, "bottom-changeset-in-batch-1")
+ (PAGE_SIZE - 1).times do
+ create_visible_changeset(user, "next-changeset")
+ end
+
+ visit "#{user_path(user)}/history"
+ changesets = find "div.changesets"
+ changesets.assert_text "bottom-changeset-in-batch-1"
+ changesets.assert_no_text "bottom-changeset-in-batch-2"
+ changesets.assert_no_text "first-changeset-in-history"
+ changesets.assert_selector "ol", :count => 1
+ changesets.assert_selector "li", :count => PAGE_SIZE
+
+ changesets.find(".changeset_more a.btn").click
+ changesets.assert_text "bottom-changeset-in-batch-1"
+ changesets.assert_text "bottom-changeset-in-batch-2"
+ changesets.assert_no_text "first-changeset-in-history"
+ changesets.assert_selector "ol", :count => 1
+ changesets.assert_selector "li", :count => 2 * PAGE_SIZE
+
+ changesets.find(".changeset_more a.btn").click
+ changesets.assert_text "bottom-changeset-in-batch-1"
+ changesets.assert_text "bottom-changeset-in-batch-2"
+ changesets.assert_text "first-changeset-in-history"
+ changesets.assert_selector "ol", :count => 1
+ changesets.assert_selector "li", :count => (2 * PAGE_SIZE) + 1
+ end
+
+ def create_visible_changeset(user, comment)
+ create(:changeset, :user => user, :num_changes => 1) do |changeset|
+ create(:changeset_tag, :changeset => changeset, :k => "comment", :v => comment)
+ end
+ end
end