Skip to content

Conversation

@zentol
Copy link
Contributor

@zentol zentol commented Feb 12, 2019

What is the purpose of the change

2 issues are being addressed here:

  • The configuration docs completeness test was parsing the document using html syntax, which has no support for self-closing tags. As a result the comparison of descriptions that contained linebreaks always failed, since we expected a self-closing tag (<br/>) but were returned something else (<br>).
  • Jsoup (which we use for parsing html) always adds a space in self-closing tags, (i.e., <br /> instead of <br/>). Functionally these behave the same, and since the rest of our documentation also includes a space I have modified the HtmlFormatter accordingly.

@flinkbot
Copy link
Collaborator

Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community
to review your pull request. We will use this comment to track the progress of the review.

Review Progress

  • ❌ 1. The [description] looks good.
  • ❌ 2. There is [consensus] that the contribution should go into to Flink.
  • ❔ 3. Needs [attention] from.
  • ❌ 4. The change fits into the overall [architecture].
  • ❌ 5. Overall code [quality] is good.

Please see the Pull Request Review Guide for a full explanation of the review process.

Details

Bot commands The @flinkbot bot supports the following commands:
  • @flinkbot approve description to approve the 1st aspect (similarly, it also supports the consensus, architecture and quality keywords)
  • @flinkbot approve all to approve all aspects
  • @flinkbot attention @username1 [@username2 ..] to require somebody's attention
  • @flinkbot disapprove architecture to remove an approval

Copy link
Contributor

@NicoK NicoK left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good, except for the commit message which is misleading, maybe it should be two commits, one hotfix and then fix fix for FLINK-11584

@zentol zentol merged commit 9839126 into apache:master Feb 12, 2019
@zentol zentol deleted the 11584 branch February 12, 2019 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants