Clojure

Added queue, queue* and queue? to clojure.core

Details

  • Type: Enhancement Enhancement
  • Status: Open Open
  • Priority: Trivial Trivial
  • Resolution: Unresolved
  • Affects Version/s: Release 1.5
  • Fix Version/s: None
  • Component/s: None
  • Patch:
    Code and Test

Description

Original patch added functions for PersistentQueue. queue, queue? and queue* match the list functions of the same naming conventions. Patches include updates to tests.

In the updated patch, "queue" accepts a single collection argument as per Rich's suggestion, and does not provide a "queue*".

Activity

Hide
Andy Fingerhut added a comment -

Timothy, I tried applying both of these Sep 26, 2012 patches to latest Clojure master as of that date. I had to apply 0001-make-PersistentQueue-ctor-public.patch by hand since it failed to apply using git or patch. It built fine, but failed to pass several of the Clojure tests. Have you looked into those test failures to see if you can find the cause and fix them? I tested on Ubuntu 11.10 with Oracle JDK 1.6 and 1.7, and saw similar failures with both.

Show
Andy Fingerhut added a comment - Timothy, I tried applying both of these Sep 26, 2012 patches to latest Clojure master as of that date. I had to apply 0001-make-PersistentQueue-ctor-public.patch by hand since it failed to apply using git or patch. It built fine, but failed to pass several of the Clojure tests. Have you looked into those test failures to see if you can find the cause and fix them? I tested on Ubuntu 11.10 with Oracle JDK 1.6 and 1.7, and saw similar failures with both.
Hide
Timothy Baldridge added a comment -

Fixed the patch. Tests pass, created the patch, applied it to a different copy of the source and the tests still pass. So this new patch should be good to go.

Show
Timothy Baldridge added a comment - Fixed the patch. Tests pass, created the patch, applied it to a different copy of the source and the tests still pass. So this new patch should be good to go.
Hide
Andy Fingerhut added a comment -

Timothy, I'm not sure how you are getting successful results when applying this patch. Can you try the steps below and see what happens for you? I get errors trying to apply the patch with latest Clojure master as of Oct 26, 2012. Also please use the steps on the JIRA workflow page to create a git format patch (http://dev.clojure.org/display/design/JIRA+workflow under "Development" heading).

% git clone git://github.com/clojure/clojure.git
% cd clojure
% patch -p1 < queues.patch
patching file src/clj/clojure/core.clj
patching file src/jvm/clojure/lang/PersistentQueue.java
Hunk #1 FAILED at 32.
1 out of 1 hunk FAILED – saving rejects to file src/jvm/clojure/lang/PersistentQueue.java.rej
patching file test/clojure/test_clojure/data_structures.clj
Hunk #1 succeeded at 123 with fuzz 2.
Hunk #2 succeeded at 861 with fuzz 2.
Hunk #3 FAILED at 872.
1 out of 3 hunks FAILED – saving rejects to file test/clojure/test_clojure/data_structures.clj.rej
patching file test/clojure/test_clojure/java_interop.clj

Show
Andy Fingerhut added a comment - Timothy, I'm not sure how you are getting successful results when applying this patch. Can you try the steps below and see what happens for you? I get errors trying to apply the patch with latest Clojure master as of Oct 26, 2012. Also please use the steps on the JIRA workflow page to create a git format patch (http://dev.clojure.org/display/design/JIRA+workflow under "Development" heading). % git clone git://github.com/clojure/clojure.git % cd clojure % patch -p1 < queues.patch patching file src/clj/clojure/core.clj patching file src/jvm/clojure/lang/PersistentQueue.java Hunk #1 FAILED at 32. 1 out of 1 hunk FAILED – saving rejects to file src/jvm/clojure/lang/PersistentQueue.java.rej patching file test/clojure/test_clojure/data_structures.clj Hunk #1 succeeded at 123 with fuzz 2. Hunk #2 succeeded at 861 with fuzz 2. Hunk #3 FAILED at 872. 1 out of 3 hunks FAILED – saving rejects to file test/clojure/test_clojure/data_structures.clj.rej patching file test/clojure/test_clojure/java_interop.clj
Hide
Timothy Baldridge added a comment -

I was using git apply. I tried the method you show above, and now I'm seeing the same issues you show above.

Show
Timothy Baldridge added a comment - I was using git apply. I tried the method you show above, and now I'm seeing the same issues you show above.
Hide
Andy Fingerhut added a comment -

Just so you know, the preferred way to create and apply patches are the "git format-patch master --stdout > patch.txt" to create a patch (after doing the branching commands described on the JIRA workflow page to create a branch for your changes), and the "git am --keep-cr -s < patch.txt" to apply a patch. If a patch was created that way and applies cleanly with that command, then you are definitely good to go.

The "patch -p1 < patch.txt" command is just a secondary method sometimes used to try to apply patches that aren't in the format produced above, or have errors when applying using that method.

Show
Andy Fingerhut added a comment - Just so you know, the preferred way to create and apply patches are the "git format-patch master --stdout > patch.txt" to create a patch (after doing the branching commands described on the JIRA workflow page to create a branch for your changes), and the "git am --keep-cr -s < patch.txt" to apply a patch. If a patch was created that way and applies cleanly with that command, then you are definitely good to go. The "patch -p1 < patch.txt" command is just a secondary method sometimes used to try to apply patches that aren't in the format produced above, or have errors when applying using that method.
Hide
Timothy Baldridge added a comment -

Just so you know, the preferred way to create and apply patches are the "git format-patch master --stdout > patch.txt" to create a patch (after doing the branching commands described on the JIRA workflow page to create a branch for your changes), and the "git am --keep-cr -s < patch.txt" to apply a patch. If a patch was created that way and applies cleanly with that command, then you are definitely good to go.

The "patch -p1 < patch.txt" command is just a secondary method sometimes used to try to apply patches that aren't in the format produced above, or have errors when applying using that method.

Show
Timothy Baldridge added a comment - Just so you know, the preferred way to create and apply patches are the "git format-patch master --stdout > patch.txt" to create a patch (after doing the branching commands described on the JIRA workflow page to create a branch for your changes), and the "git am --keep-cr -s < patch.txt" to apply a patch. If a patch was created that way and applies cleanly with that command, then you are definitely good to go. The "patch -p1 < patch.txt" command is just a secondary method sometimes used to try to apply patches that aren't in the format produced above, or have errors when applying using that method.
Hide
Timothy Baldridge added a comment - - edited

added patch

Show
Timothy Baldridge added a comment - - edited added patch
Hide
Andy Fingerhut added a comment -

That one applies cleanly and passes all tests. It should show up on the next list of prescreened patches. Thanks.

Show
Andy Fingerhut added a comment - That one applies cleanly and passes all tests. It should show up on the next list of prescreened patches. Thanks.
Hide
Rich Hickey added a comment -

we don't use the queue* convention elsewhere, e.g. vec and vector. I think queue should take a collection like vec and set. (queue [1 2 3]) could be made to 'adopt' the collection as front.

Show
Rich Hickey added a comment - we don't use the queue* convention elsewhere, e.g. vec and vector. I think queue should take a collection like vec and set. (queue [1 2 3]) could be made to 'adopt' the collection as front.
Hide
Andy Fingerhut added a comment -

Patch queue.patch dated Oct 26 2012 no longer applies cleanly after recent CLJ-1000 commits, but only because of one line of changed patch context. It still applies cleanly with "patch -p1 < queue.patch". Not bothering to update the stale patch given Rich's comments suggesting more substantive changes.

Show
Andy Fingerhut added a comment - Patch queue.patch dated Oct 26 2012 no longer applies cleanly after recent CLJ-1000 commits, but only because of one line of changed patch context. It still applies cleanly with "patch -p1 < queue.patch". Not bothering to update the stale patch given Rich's comments suggesting more substantive changes.
Hide
Steve Miner added a comment -

See also CLJ-976 (tagged literal support for PersistentQueue)

Show
Steve Miner added a comment - See also CLJ-976 (tagged literal support for PersistentQueue)
Hide
John Jacobsen added a comment -

Don't want to step on Timothy B's toes here, but it looks straightforward to adopt his patch to implement Rich's suggestion. I'd offer to give it a whack if nobody else wants the ticket now.

Show
John Jacobsen added a comment - Don't want to step on Timothy B's toes here, but it looks straightforward to adopt his patch to implement Rich's suggestion. I'd offer to give it a whack if nobody else wants the ticket now.
Hide
John Jacobsen added a comment -
Show
John Jacobsen added a comment - Discussion initiated on clojure-dev: https://groups.google.com/forum/?fromgroups#!topic/clojure-dev/2BOqHm24Vc4
Hide
John Jacobsen added a comment -

This patch (if accepted) supersedes Timothy Baldridge's patch; it implements "queue" and "queue?" (but not "queue*"); "queue" accepts a collection rather than being a variadic function, as per Rich's suggestion.

Show
John Jacobsen added a comment - This patch (if accepted) supersedes Timothy Baldridge's patch; it implements "queue" and "queue?" (but not "queue*"); "queue" accepts a collection rather than being a variadic function, as per Rich's suggestion.

People

Vote (3)
Watch (3)

Dates

  • Created:
    Updated: