Skip to main content

[pkg-discuss] Re: Code review of pkg search bugs 15620551 and 16190165

  • From: Shawn Walker < >
  • To: "Thejaswini.K" < >
  • Cc:
  • Subject: [pkg-discuss] Re: Code review of pkg search bugs 15620551 and 16190165
  • Date: Thu, 01 Aug 2013 11:37:13 -0700

On 08/01/13 11:30, Thejaswini.K wrote:
On Wednesday 31 July 2013 10:15 PM, Shawn Walker wrote:
On 07/30/13 21:32, Thejaswini wrote:
Hi Shawn

The new webrev is @ http://ips.java.net/webrev/tk241774/15620551/rev04/

src/client.py:
  line 2674: This should be the same regex used in actions/depend.py
    so that the publisher prefix will be stripped too.
But then query like "pkg search pkg://pub1/pkg1" will list pkg1 present
in all publishers.

Yes, it will. But right now, it doesn't return any results anyway. So this would actually be an improvement.

Search doesn't handle publishers, and that's ok.

Realistically, I don't think we need to support publisher-specific search this way.

You don't need
    to use the compile() logic described below though; just use the
    existing re.sub() logic here.
  line 319: Because this regex will be used multiple times,
    it should be compiled into a RegexObject and then reused.
    To do that, on line 314 (after the inds = []):

      pat = re.compile(r"pkg:///|pkg://[^/]*/|pkg:/")

    ...and then on line 319:

      p = pat.sub("", p)

src/tests/cli/t_pkg_search.py:
  line 1056, 1066, 1078: Add "-H" to the list of options.

  lines 1052, 1063, 1072: drop these lines

  lines 1060, 1069, 1081: s/assertEqual/assertEqualDiff/
line 1082: Insert another newline here
I have made these changes and the webrev is at
http://ips.java.net/webrev/tk241774/15620551/rev06/

src/tests/cli/t_pkg_search.py:
  line 1061: s/out2=/out2 =/

Otherwise, this looks fine. You don't need to email another webrev out; just push when you're ready.

-Shawn



[pkg-discuss] Re: Code review of pkg search bugs 15620551 and 16190165

Thejaswini.K 08/01/2013

[pkg-discuss] Re: Code review of pkg search bugs 15620551 and 16190165

Shawn Walker 08/01/2013
 
 
Close
loading
Please Confirm
Close