Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DOXIA-670] Drop dead XdocSink#link(String, String) method #116

Merged
merged 1 commit into from Oct 9, 2022

Conversation

michael-o
Copy link
Member

@michael-o michael-o commented Aug 4, 2022

@michael-o michael-o requested a review from hboutemy August 4, 2022 20:10
@asfgit asfgit force-pushed the DOXIA-670 branch 2 times, most recently from 22fdc61 to 0f10501 Compare August 4, 2022 20:12
@michael-o
Copy link
Member Author

michael-o commented Aug 5, 2022

Note: This change requires to be dropped:

<xs:element name="source">
<xs:annotation>
<xs:documentation source="version">2.0.0</xs:documentation>
<xs:documentation source="description">
A source element.
</xs:documentation>
</xs:annotation>
<xs:complexType mixed="true">
<xs:sequence>
<xs:any minOccurs="0" maxOccurs="unbounded" processContents="skip"/>
</xs:sequence>
</xs:complexType>
</xs:element>
and release a new version. Alternatively, we'd translate source to <div class="source"><pre>...

@asfgit asfgit force-pushed the DOXIA-670 branch 2 times, most recently from 66f4028 to e6514f5 Compare August 20, 2022 20:32
@michael-o
Copy link
Member Author

This is now a minimalist version. I will leave the table as is.

Copy link
Member

@hboutemy hboutemy left a comment

Choose a reason for hiding this comment

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

let's rename this issue into "drop dead link(String, String) code" and have VALIGN removal in a separate issue, with discussion on source also separate


writeStartTag( A, att );
}

Copy link
Member

Choose a reason for hiding this comment

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

it seems that this link(String, String) method is more than "duplicate code": it's dead code that was introduced into Doxia 1.1 and was never called outside the unit test
then I'm all in to remove this dead code and update the associated unit test, with proper Jira issue renaming

on other changes like VALIGN, that are unrelated to this dead code removal, I'd prefer have a separate commit and review to better understand

and I don't understand why there is a discussion on "source", which seems to be completely an unrelated discussion

Copy link
Member Author

Choose a reason for hiding this comment

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

@hboutemy Done, completely reduced. Attributes are in another ticket. As for the source. HTML5 has now a source element which collides with Xdoc's source. That is why I decided not to fiddle with at the moment. Forget about it.

asfgit pushed a commit that referenced this pull request Oct 1, 2022
@michael-o michael-o changed the title [DOXIA-670] Remove code duplication in XdocSink [DOXIA-670] Drop dead XdocSink#link(String, String) method Oct 1, 2022
@michael-o
Copy link
Member Author

@hboutemy PR redone.

Copy link
Member

@hboutemy hboutemy left a comment

Choose a reason for hiding this comment

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

thanks for the hard work

@asfgit asfgit closed this in 1b2e9a9 Oct 9, 2022
@asfgit asfgit merged commit 1b2e9a9 into master Oct 9, 2022
@michael-o michael-o deleted the DOXIA-670 branch October 9, 2022 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants