Skip to main content

[pkg-discuss] Re: Code review request:16273625 case-insensitive search broken

  • From: Thejaswini < >
  • To:
  • Subject: [pkg-discuss] Re: Code review request:16273625 case-insensitive search broken
  • Date: Thu, 08 Aug 2013 09:04:09 +0530

<html>
  <head>
    <meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    <br>
    <div class="moz-signature">
      <title></title>
      <img src="cid:part1.05090906.03060207@oracle.com" alt="Oracle"
        height="30" border="0" width="110"><br>
      Thejaswini K<br>
      Revenue Product Engineering (RPE), Systems <br>
      Phone: +91 8067283833 | Mobile: +91 9663324594 <font
        color="#666666" face="Verdana, Arial, Helvetica, sans-serif"
        size="2"><br>
        ORACLE India | Off Langford Road | Bangalore | 560025 </font>
      <br>
      <a href="http://www.oracle.com/commitment"; target="_blank"><img
          src="cid:part2.07050907.07030106@oracle.com" alt="Green
          Oracle" height="28" border="0" width="44" align="middle"></a>
      <font color="#4b7d42" face="Verdana, Arial, Helvetica, sans-serif"
        size="1">Oracle is committed to developing practices and
        products that
        help protect the environment</font>
      <!-- This signature was generated by the MyDesktop Oracle Business Signature utility -->
    </div>
    <br>
    On 08/08/13 00:09, Shawn Walker wrote:
    <blockquote cite="mid:
      " type="cite">On
      08/07/13 11:32, Shawn Walker wrote:
      <br>
      <blockquote type="cite">On 08/07/13 01:46, Thejaswini wrote:
        <br>
        <blockquote type="cite">
          <br>
          On 08/07/13 00:11, Shawn Walker wrote:
          <br>
          <blockquote type="cite">On 08/05/13 23:03, Thejaswini wrote:
            <br>
            <blockquote type="cite">Hi All,
              <br>
              <br>
              The webrev
              <a class="moz-txt-link-freetext" href="http://ips.java.net/webrev/tk241774/case-search/rev01/";>http://ips.java.net/webrev/tk241774/case-search/rev01/</a>
              <br>
              has fix for 16273625
              <a class="moz-txt-link-rfc2396E" href="https://bug.oraclecorp.com/show?16273625";>&lt;https://bug.oraclecorp.com/show?16273625&gt;</a>
              <br>
              case-insensitive search broken for specific queries.
              <br>
              <br>
              The fix is only two line change in query_parser.py.
              <br>
              <br>
              I ran the complete test-suite and all the test-cases pass.
              <br>
              <br>
              Please review it and let me know your
              comments/suggestions.
              <br>
            </blockquote>
            <br>
            src/modules/query_parser.py:
            <br>
              line 1415: I see you added the call _close_dicts(), but
            <br>
                it's missing the important comment found on lines
            1521-1522
            <br>
                that says why it is calling it.
            <br>
            <br>
              line 1416: Add a newline after this
            <br>
          </blockquote>
          <br>
          I have incorporated the above changes and the webrev is @
          <br>
          <a class="moz-txt-link-freetext" href="http://ips.java.net/webrev/tk241774/case-search/rev02/";>http://ips.java.net/webrev/tk241774/case-search/rev02/</a>
          <br>
          <blockquote type="cite">
            <br>
            src/tests/cli/t_pkg_search.py:
            <br>
              Does this test case fail without the changes to
            <br>
            src/modules/query_parser.py?
            <br>
          </blockquote>
          Yes it failed. Therefore I have added a assert statement to
          make sure
          <br>
          there are no error messages thrown.
          <br>
          <br>
          If there are no more comments I would push the changes
          tomorrow.
          <br>
        </blockquote>
      </blockquote>
      <br>
      Actually, the assert() doesn't make any sense.
      <br>
      <br>
      I would expect self.errout to containg something if the commands
      exits with 1.
      <br>
    </blockquote>
    The command exits with 1 as there are no search results.<br>
    <blockquote cite="mid:
      " type="cite">
      <br>
      What does self.output and self.stderr contain at that point?
      <br>
    </blockquote>
    self.output and self.errout both are Null as there are no search
    results.<br>
    <blockquote cite="mid:
      " type="cite">
      <br>
      Does the test case actually succeed after you added the assert?
      <br>
    </blockquote>
    Yes the test succeeds after adding the assert and fails if I remove
    the changes in query_parser.py.<br>
    Thanks,<br>
    Thejaswini K.<br>
    <blockquote cite="mid:
      " type="cite">
      <br>
      -Shawn
      <br>
      <br>
    </blockquote>
  </body>
</html>

GIF image

GIF image



[pkg-discuss] Code review request:16273625 case-insensitive search broken

Thejaswini 08/06/2013

[pkg-discuss] Re: Code review request:16273625 case-insensitive search broken

Shawn Walker 08/06/2013

[pkg-discuss] Re: Code review request:16273625 case-insensitive search broken

Thejaswini 08/07/2013

[pkg-discuss] Re: Code review request:16273625 case-insensitive search broken

Shawn Walker 08/07/2013

[pkg-discuss] Re: Code review request:16273625 case-insensitive search broken

Shawn Walker 08/07/2013

[pkg-discuss] Re: Code review request:16273625 case-insensitive search broken

Thejaswini 08/08/2013

[pkg-discuss] Re: Code review request:16273625 case-insensitive search broken

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