#315: Proper implementation of par template (with recursion) #359

paulodamaso · 2018-06-29T20:18:15Z

For #315, made proper implementation of par template (with recursion)
Performed changes:

Corrected par template in templates.xsl for using recursion and correctly replacing paragraphs numbers;
Added comment to par.xsl, corrected test description and set ignore to false.

- Added `style` attribute to `code` element in `expected` parameter in `project.xsl`; - Replaced `!=` operator by `not(deep-equal())` in `assertions.xsl` because received parameters are document fragments, not strings; - Added `xmlns:xs` so it can be excluded in `assert-that` comparisons (`project.xsl` puts `xmlns:xs` in its elements); - Corrected `actual` and `expected` variable names in failure message.

Changes: - Corrected `par` template in `templates.xsl` for using recursion and correctly replacing paragraphs numbers; - Added comment to `par.xsl`, corrected test description and set `ignore` to `false`.

0crat · 2018-06-29T20:18:26Z

Job #359 is now in scope, role is REV

0crat · 2018-06-29T20:24:04Z

This pull request #359 is assigned to @amihaiemil /z, here is why; the budget is 15 minutes, see §4; please, read §27 and when you decide to accept the changes, inform @g4s8 /z (the architect) right in this ticket; if you decide that this PR should not be accepted ever, also inform the architect; this blog post will help you understand what is expected from a code reviewer; there will be a monetary reward for this job

paulodamaso · 2018-06-29T20:24:41Z

@amihaiemil Had to commit #339 changes into this branch, because they are needed to proper behavior of assert-that template. Oh and don't be impressed with travis error, it's is segfaulting a lot today.

amihaiemil · 2018-06-30T06:50:52Z

@paulodamaso I'll have a look in 1-2 days, I'm away from my laptop for the weekend :(

amihaiemil

@paulodamaso A few comments, please have a look. And try to see why CI is failing -- from what I see, it has nothing to do with your changes but maybe I'm missing something.

amihaiemil · 2018-07-01T17:27:23Z

xsl/templates.xsl

+    <xsl:analyze-string select="$text" regex="(§[0-9]*)">
+      <xsl:matching-substring>
+        <xsl:variable select="regex-group(1)" name="paragraph" />
+        <a href="{concat('http://www.zerocracy.com/policy.html#', substring-after($paragraph,'§'))}">


@paulodamaso We should use https:// here, I see it is enabled for zerocracy.com, but redirection from http:// to https:// is not :(

amihaiemil · 2018-07-01T17:28:31Z

xsl-test/templates/project.xsl

@@ -23,8 +23,9 @@ SOFTWARE.
      <xsl:with-param name="message" select="'Fails to format project name'"/>
      <xsl:with-param name="expected">
        <span>
-          <code>
-            <a href="https://www.0crat.com/p/ABCDEFGHI" title="Project ABCDEFGHI">
+          <code style="background-color:#daf1e0;">


@paulodamaso Why the background-color attribute, what does it do? And is the reindentation of <a> needed?

@amihaiemil This bug correction depends on #339; I've had to put it here so the build doesn't fali.

amihaiemil · 2018-07-01T17:29:23Z

xsl-test/assertions.xsl

          <xsl:text>" is not equal to expected "</xsl:text>
-          <xsl:value-of select="$expected_xml"/>
+          <xsl:copy-of select="$expected"/>
          <xsl:text>")</xsl:text>
        </xsl:message>
      </xsl:if>


@paulodamaso I don't think the changes to this file belong in this PR, or?

@amihaiemil Same as above.

Usage of `https` instead `http`.

paulodamaso · 2018-07-02T02:02:48Z

@amihaiemil Thanks for the comments, made some changes and answered question, please take a look.

…256#339.

paulodamaso · 2018-07-02T02:56:44Z

@amihaiemil Let's do this the right way. I've removed all changes unrelated to #315, but now the build fails because some wrong code in master.

amihaiemil · 2018-07-02T07:02:24Z

@paulodamaso yes, the most correct would be to put this task on hold before #339 is solved. If you can merge the PR for #339 soon (the next 5 days?), then we can wait with this PR.

But if you think it will take longer, then we should close this PR (you keep your branch), and make another one later after the impediment is merged. This is so I don't lose this job, because I can't put PRs on hold ;(

paulodamaso · 2018-07-02T11:59:33Z

@amihaiemil Don't worry, I'll try to fix #339 build problems as soon as possible. I think we will have enough time to this PR.

amihaiemil · 2018-07-02T12:08:18Z

@paulodamaso sure, thanks :D

amihaiemil · 2018-07-03T07:51:25Z

@paulodamaso I think you merged the waiting fix, right? But now the CI is still failing :D

paulodamaso · 2018-07-03T14:19:49Z

@amihaiemil I'm having some problems with the build, seems like some lib we are using does not support XSLT 2.0 analyze-string tag. The tests runs locally in my IDE, using Saxon 9.8.08 as processor, I'm suspecting that is a Nokogiri problem. Fortunately it does not work either with Nokogiri 1.8, so it was not caused by the downgrade of Nokogiri in #357 .

paulodamaso · 2018-07-03T18:11:51Z

@g4s8 @amihaiemil This is a no-go here. Nokogiri does not support XPath 2.0 and XSLT 2.0 (see https://stackoverflow.com/questions/2838320/nokogiri-ruby-and-xpath and https://www.rubydoc.info/github/sparklemotion/nokogiri) because it uses libxslt which is an very very old lib. I don't think there is an elegant way of doing this recursive search using XSL 1.0, compared to analyze-string usage; so, I think we should try another way of building these files instead using Nokogiri, WDYT?

amihaiemil · 2018-07-04T07:47:18Z

@paulodamaso @g4s8 I honestly have no idea, but this seems like a bug. I would say: file a bug so you are paid for this research, put your task on hold (this one) until that one is fixed and you have all the necessary tools to remake this PR later. WDYT?

g4s8 · 2018-07-04T12:18:43Z

@paulodamaso @amihaiemil right, I think we have to use Saxon to transform xsl (like it transformed to html in Rakefile), @paulodamaso please submit a bug to run xsl tests with Saxon instead of Nokogiri.

paulodamaso · 2018-07-04T14:15:56Z

@g4s8 Thanks for the direction, created #365 for resolving the issue. Will correct this PR to ignore the test while the new bug is not corrected.

paulodamaso · 2018-07-04T14:31:58Z

@g4s8 Closed PR and set impediment in #315.

0crat · 2018-07-04T14:49:12Z

@ypshenychka /z please review this job completed by @amihaiemil /z, as in §30; the job will be fully closed and all payments will be made when the quality review is completed

0crat · 2018-07-04T14:49:32Z

The job #359 is now out of scope

0crat · 2018-07-04T14:49:51Z

Payment to ARC for a closed pull request, as in §28: +10 point(s) just awarded to @g4s8 /z

ypshenychka · 2018-07-04T18:43:03Z

@amihaiemil According to our QA Rules:

The code reviewer found at least three problems in the code.
Comments were mostly about design problems, not cosmetic issues.

Only two issues were found during code review.
Please confirm that you'll try to find at least three major problems while future reviews.

amihaiemil · 2018-07-04T18:47:08Z

@ypshenychka confirmed

ypshenychka · 2018-07-04T18:47:50Z

@amihaiemil Thanks

ypshenychka · 2018-07-04T18:47:56Z

@0crat quality acceptable

0crat · 2018-07-04T18:56:30Z

Order was finished, quality is "acceptable": +15 point(s) just awarded to @amihaiemil /z

ypshenychka · 2018-07-04T19:30:03Z

@0crat status

0crat · 2018-07-04T19:30:15Z

@0crat status (here)

@ypshenychka This is what I know about this job in C3NDPUA8L, as in §32:

The job #359 is not in scope
Job footprint (restricted area)

ypshenychka · 2018-07-04T19:37:24Z

@g4s8 For some reason I got no points and payment here. Could you please do it manually?

ypshenychka · 2018-07-06T06:43:21Z

@g4s8 ping

paulodamaso added 4 commits June 29, 2018 00:39

For yegor256#339: Correction suggested in review.

ac5af1d

Merge branch '339' into 315

fb77805

For yegor256#315: Correction of par template

4054e8e

Changes: - Corrected `par` template in `templates.xsl` for using recursion and correctly replacing paragraphs numbers; - Added comment to `par.xsl`, corrected test description and set `ignore` to `false`.

0crat added the scope label Jun 29, 2018

krzyk added 3 commits June 30, 2018 15:23

Inspector image instead of login

83b920a

No image when no inspector

34cfa3d

corrected choose

080c4b0

amihaiemil reviewed Jul 1, 2018

View reviewed changes

For yegor256#359 review.

7fab082

Usage of `https` instead `http`.

paulodamaso added 4 commits July 1, 2018 23:19

For yegor256#339: End of line corrections.

b0d8284

For yegor256#339: End of line corrections.

cb3a76e

For yegor256#339: XSL corrections.

8cd2e91

For yegor256#315: Correcting line endings, removed changes from yegor…

329512c

…256#339.

krzyk added 2 commits July 2, 2018 12:43

moving project to job column

a328ea4

remove also the header for project column

dc81ac1

g4s8 and others added 5 commits July 2, 2018 17:33

downgraded nokogiri

53909c3

Merge branch '__rultor'

3662e58

For yegor256#339: Downgrade of nokogiri, update from master.

44d7d2d

Merge branch '__rultor'

b8e7648

For yegor256#315: Updated files from master.

26326ec

Merge branch '315'

449804a

For yegor256#339: Ignore par tests due lack of XSL 2.0 support.

5f76663

paulodamaso closed this Jul 4, 2018

0crat removed the scope label Jul 4, 2018

paulodamaso deleted the 315 branch July 10, 2018 03:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#315: Proper implementation of par template (with recursion) #359

#315: Proper implementation of par template (with recursion) #359

paulodamaso commented Jun 29, 2018

0crat commented Jun 29, 2018

0crat commented Jun 29, 2018

paulodamaso commented Jun 29, 2018 •

edited

Loading

amihaiemil commented Jun 30, 2018

amihaiemil left a comment

amihaiemil Jul 1, 2018

amihaiemil Jul 1, 2018

paulodamaso Jul 2, 2018

amihaiemil Jul 1, 2018

paulodamaso Jul 2, 2018

paulodamaso commented Jul 2, 2018

paulodamaso commented Jul 2, 2018

amihaiemil commented Jul 2, 2018 •

edited

Loading

paulodamaso commented Jul 2, 2018

amihaiemil commented Jul 2, 2018

amihaiemil commented Jul 3, 2018

paulodamaso commented Jul 3, 2018

paulodamaso commented Jul 3, 2018 •

edited

Loading

amihaiemil commented Jul 4, 2018

g4s8 commented Jul 4, 2018

paulodamaso commented Jul 4, 2018

paulodamaso commented Jul 4, 2018

0crat commented Jul 4, 2018

0crat commented Jul 4, 2018

0crat commented Jul 4, 2018

ypshenychka commented Jul 4, 2018

amihaiemil commented Jul 4, 2018

ypshenychka commented Jul 4, 2018

ypshenychka commented Jul 4, 2018

0crat commented Jul 4, 2018

ypshenychka commented Jul 4, 2018

0crat commented Jul 4, 2018

ypshenychka commented Jul 4, 2018

ypshenychka commented Jul 6, 2018

#315: Proper implementation of par template (with recursion) #359

#315: Proper implementation of par template (with recursion) #359

Conversation

paulodamaso commented Jun 29, 2018

0crat commented Jun 29, 2018

0crat commented Jun 29, 2018

paulodamaso commented Jun 29, 2018 • edited Loading

amihaiemil commented Jun 30, 2018

amihaiemil left a comment

Choose a reason for hiding this comment

amihaiemil Jul 1, 2018

Choose a reason for hiding this comment

amihaiemil Jul 1, 2018

Choose a reason for hiding this comment

paulodamaso Jul 2, 2018

Choose a reason for hiding this comment

amihaiemil Jul 1, 2018

Choose a reason for hiding this comment

paulodamaso Jul 2, 2018

Choose a reason for hiding this comment

paulodamaso commented Jul 2, 2018

paulodamaso commented Jul 2, 2018

amihaiemil commented Jul 2, 2018 • edited Loading

paulodamaso commented Jul 2, 2018

amihaiemil commented Jul 2, 2018

amihaiemil commented Jul 3, 2018

paulodamaso commented Jul 3, 2018

paulodamaso commented Jul 3, 2018 • edited Loading

amihaiemil commented Jul 4, 2018

g4s8 commented Jul 4, 2018

paulodamaso commented Jul 4, 2018

paulodamaso commented Jul 4, 2018

0crat commented Jul 4, 2018

0crat commented Jul 4, 2018

0crat commented Jul 4, 2018

ypshenychka commented Jul 4, 2018

amihaiemil commented Jul 4, 2018

ypshenychka commented Jul 4, 2018

ypshenychka commented Jul 4, 2018

0crat commented Jul 4, 2018

ypshenychka commented Jul 4, 2018

0crat commented Jul 4, 2018

ypshenychka commented Jul 4, 2018

ypshenychka commented Jul 6, 2018

paulodamaso commented Jun 29, 2018 •

edited

Loading

amihaiemil commented Jul 2, 2018 •

edited

Loading

paulodamaso commented Jul 3, 2018 •

edited

Loading