Code review comment for lp:~barry/launchpad/rnr-techdebt

Revision history for this message
Curtis Hovey (sinzui) wrote :

Hi Barry.

I really appreciate your attention to this issue. I am happy you asked me
to review this because I am familiar with the i18n Answers issues
in the test suite. Your diff must pass this test to be safe to commit.

    iconv -t ascii ~/Desktop/barry.diff > /dev/null

Launchpad code is not i18n enabled, Answers and my editor is! I see
the Arabic portions of the diff right to left. That is very exciting
to see, but made scanning impossible, and when I look at the doctest...well
wow, I cannot read that. I asked Björn read it and his editor refused to
display the file.

You can use .encode .decode to make the words ascii, or use asciismash()
from somewhere in our code. I used both of these when I had to print the
words out. I got tired of that so I just showed the representation.
You can also use ellipsis if you need to hide an awkward part.

You will see a lot of duplication in these tests. I was young and naive
when I wrote them. You can consider deleting any duplication you see when
you fix the encoding issues. There is a small chance that there is a model
or view test that is the definitive test for many of these stories (At least
most of the answers tests are stories)

I see Spanish and Arabic in the diff, I was sure there was some French in
the tests I wrote, but you may not have touched those tests.

> === modified file 'lib/lp/answers/doc/person.txt'
> --- lib/lp/answers/doc/person.txt 2009-12-24 01:41:54 +0000
> +++ lib/lp/answers/doc/person.txt 2010-02-05 12:29:14 +0000
...
> +Language
> +--------
...
> +But Carlos has one.
>
> >>> carlos_raw = personset.getByName('carlos')
> >>> carlos = IQuestionsPerson(carlos_raw)
> >>> for question in carlos.searchQuestions(
> - ... language=[english, spanish]):
> - ... [question.title, question.language.code]
> - [u'Problema al recompilar kernel con soporte smp (doble-n\xfacleo)',
> - u'es']
> -
> -
> -=== needs_attention ===
> -
> -The method accept a parameter called needs_attention which only selects
> -the questions that needs attention from the person. This includes questions
> -owned by the person in the ANSWERED or NEEDSINFO state. It also includes
> -questions on which the person requested for more information or gave an
> -answer and that are back in the OPEN state.
> + ... language=(english, spanish)):
> + ... print question.title, question.language.code
> + Problema al recompilar kernel con soporte smp (doble-núcleo) es

...

> === renamed file 'lib/lp/answers/doc/utility.txt' => 'lib/lp/answers/doc/questionsets.txt'
> --- lib/lp/answers/doc/utility.txt 2009-03-24 12:43:49 +0000
> +++ lib/lp/answers/doc/questionsets.txt 2010-02-05 12:29:14 +0000...

...

> +Searching questions
> +===================
> +
> +The IQuestionSet interface defines a searchQuestions() method that is used to
> +search for questions defined in any question target.
> +
> +
> +Search text
> +-----------
> +
> +The search_text parameter will return questions matching the query using the
> +regular full text algorithm.
>
> >>> for question in question_set.searchQuestions(search_text='firefox'):
> - ... print repr(question.title), question.target.displayname
> - u'Problemas de Impress\xe3o no Firefox' Mozilla Firefox
> - u'Firefox loses focus and gets stuck' Mozilla Firefox
> - u'Firefox cannot render Bank Site' Mozilla Firefox
> - u'mailto: problem in webpage' mozilla-firefox in ubuntu
> - u"Newly installed plug-in doesn't seem to be used" Mozilla Firefox
> - u'Problem showing the SVG demo on W3C site' Mozilla Firefox
> - u'\u0639\u0643\u0633 ...' Ubuntu
> -
> -
> -=== status ===
> -
> -By default, expired and invalid questions are not searched for. The
> -status parameter can be used to select the questions in the status
> -you are interested in.
> -
> - >>> from canonical.launchpad.interfaces import QuestionStatus
> + ... print question.title, question.target.displayname
> + Problemas de Impressão no Firefox Mozilla Firefox
> + Firefox loses focus and gets stuck Mozilla Firefox
> + Firefox cannot render Bank Site Mozilla Firefox
> + mailto: problem in webpage mozilla-firefox in ubuntu
> + Newly installed plug-in doesn't seem to be used Mozilla Firefox
> + Problem showing the SVG demo on W3C site Mozilla Firefox
> + عكس التغييرات غير المحفوظة للمستن؟ Ubuntu
> +
> +
> +
> +
^ These 6 lines are right to left and flow off my screen. They are
Arabic right-to-left

...

> +Language
> +--------
>
> The language parameter can be used to select only questions written in a
> particular language.
>
> - >>> from canonical.launchpad.interfaces import ILanguageSet
> + >>> from lp.services.worlddata.interfaces.language import ILanguageSet
> >>> spanish = getUtility(ILanguageSet)['es']
> >>> for t in question_set.searchQuestions(language=spanish):
> - ... print t.title.encode('us-ascii', 'backslashreplace')
> - Problema al recompilar kernel con soporte smp (doble-n\xfacleo)
> -
> -=== Combination ===
> -
> -The returned sets of questions is the intersection of the sets delimited
> -by each criteria:
> + ... print t.title
> + Problema al recompilar kernel con soporte smp (doble-núcleo)

^ Spanish
...

> +Combinations
> +------------
> +
> +The returned set of questions is the intersection of the sets delimited by
> +each criteria.
>
> >>> for question in question_set.searchQuestions(
> ... search_text='firefox',
> - ... status=[QuestionStatus.OPEN, QuestionStatus.INVALID]):
> - ... print repr(question.title), question.status.title, (
> + ... status=(QuestionStatus.OPEN, QuestionStatus.INVALID)):
> + ... print question.title, question.status.title, (
> ... question.target.displayname)
> - u'Problemas de Impress\xe3o no Firefox' Open Mozilla Firefox
> - u'Firefox is slow and consumes too much RAM' Invalid mozilla-firefox in ubuntu
> - u'Firefox loses focus and gets stuck' Open Mozilla Firefox
> - u'Firefox cannot render Bank Site' Open Mozilla Firefox
> - u'Problem showing the SVG demo on W3C site' Open Mozilla Firefox
> - u'\u0639\u0643\u0633 ...' Open Ubuntu
> -
> -
> -=== Sort Order ===
> -
> -When using the search_text criteria, the default is to sort the results
> -by relevancy. One can use the sort parameter to change that. It takes
> -one of the constant defined in the QuestionSort enumeration.
> -
> - >>> from canonical.launchpad.interfaces import QuestionSort
> + Problemas de Impressão no Firefox Open Mozilla Firefox
> + Firefox is slow and consumes too much RAM Invalid mozilla-firefox in ubuntu
> + Firefox loses focus and gets stuck Open Mozilla Firefox
> + Firefox cannot render Bank Site Open Mozilla Firefox
> + Problem showing the SVG demo on W3C site Open Mozilla Firefox
> + عكس التغييرات غير المحفوظة للمستن؟ Open Ubuntu
> +
> +
> +
^ Spanish and Arabic right-to left

> Sort order
> +----------

^ This section listed Arabic and Spanish questions.

...

> +Search text
> +-----------
>
> The search_text parameter will select the questions that contain the
> -passed in text. (The standard text searching algorithm is used, see
> -textsearching.txt.)
> +passed in text. The standard text searching algorithm is used; see
> +../../../canonical/launchpad/doct/textsearching.txt.

^ This path does not exist.

...

review: Needs Fixing

« Back to merge proposal