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

feat(#757): Exclude roll-data.xsl Transformation From Unrolling #760

Merged
merged 1 commit into from
Oct 15, 2024

Conversation

volodya-lombrozo
Copy link
Member

@volodya-lombrozo volodya-lombrozo commented Oct 15, 2024

In this PR I excluded roll-data.xsl from the suggested unrolling train.

Related to #757


PR-Codex overview

This PR focuses on enhancing the CanonicalXmir functionality by modifying the transformation process and adding a new test case to ensure correct unrolling of sequences in an XML representation.

Detailed summary

  • Updated the TrClasspath in CanonicalXmir.java to include a new file and remove two others.
  • Added imports for Directives, ImpossibleModificationException, and Xembly in CanonicalXmirTest.java.
  • Introduced a new test method unrollsSequenceOfValuesCorrectly to validate the unrolling of values in XML.

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

@volodya-lombrozo
Copy link
Member Author

@maxonfjvipon Could you take a look, please?

Copy link
Member

@maxonfjvipon maxonfjvipon left a comment

Choose a reason for hiding this comment

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

@volodya-lombrozo Looks good, seems everything works fine without the transformation

@volodya-lombrozo
Copy link
Member Author

@rultor merge

@rultor
Copy link
Contributor

rultor commented Oct 15, 2024

@rultor merge

@volodya-lombrozo OK, I'll try to merge now. You can check the progress of the merge here.

@rultor rultor merged commit cedbc5c into objectionary:master Oct 15, 2024
11 checks passed
@rultor
Copy link
Contributor

rultor commented Oct 15, 2024

@rultor merge

@volodya-lombrozo Done! FYI, the full log is here (took me 11min).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants