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

Feature/marp 1018 new connector rtf factory #4

Merged
merged 25 commits into from
Sep 19, 2024

Conversation

tvtphuc-axonivy
Copy link
Contributor

No description provided.

@github-actions github-actions bot added the enhancement New feature or request label Sep 9, 2024
Copy link
Contributor

github-actions bot commented Sep 9, 2024

Test Results

2 tests  +2   2 ✅ +2   6s ⏱️ +6s
2 suites +2   0 💤 ±0 
2 files   +2   0 ❌ ±0 

Results for commit f8cbbd9. ± Comparison against base commit dfaa015.

♻️ This comment has been updated with latest results.

Copy link
Contributor Author

@tvtphuc-axonivy tvtphuc-axonivy left a comment

Choose a reason for hiding this comment

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

Hi @ivy-bb , I have reset the master branch to your initial commit. I cloned your code to the branch "feature/MARP-1018-New-connector-RTF-Factory"
and then created a PR to the master branch that has some feedback needed to be addressed.

rtf-factory/src/ch/ivyteam/ivy/RtfFactory/RtfExpander.java Outdated Show resolved Hide resolved
rtf-factory/src/ch/ivyteam/ivy/RtfFactory/RtfExpander.java Outdated Show resolved Hide resolved
rtf-factory/src/ch/ivyteam/ivy/RtfFactory/RtfExpander.java Outdated Show resolved Hide resolved
rtf-factory/src/ch/ivyteam/ivy/RtfFactory/RtfExpander.java Outdated Show resolved Hide resolved
rtf-factory/src/ch/ivyteam/ivy/RtfFactory/RtfExpander.java Outdated Show resolved Hide resolved

public class ExportFromCms {

public static java.io.File export(String cmsUri, String ext) throws IOException {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. Missing unit test for this method.

  2. When I click "Create welcome document" . I see the "customer.name = Flury!" . The "." symbol is valid here ?
    image

Copy link
Member

Choose a reason for hiding this comment

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

I have improved the included cms text.

rtf-factory/src/ch/ivyteam/ivy/RtfFactory/RtfExpander.java Outdated Show resolved Hide resolved
@tvtphuc-axonivy
Copy link
Contributor Author

Hi @ivy-bb I did an enhancement and format for this connector.
Could you help to review and give your feedback? then I can continue for onboarding it. Thanks!
FYI @ivy-sgi

@ivy-bb
Copy link
Member

ivy-bb commented Sep 19, 2024

Thank you very much for your good work.
I finally found the time to review. For me it looks very smart now.

@ivy-bb ivy-bb merged commit 89951cc into master Sep 19, 2024
4 checks passed
@ivy-bb ivy-bb deleted the feature/MARP-1018-New-connector-RTF-Factory branch September 19, 2024 09:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants