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

Codeunit 1483 XmlWriter lacks some overloads for WriteElementString() #346

Closed
jhoek opened this issue Oct 24, 2023 · 7 comments · Fixed by #700
Closed

Codeunit 1483 XmlWriter lacks some overloads for WriteElementString() #346

jhoek opened this issue Oct 24, 2023 · 7 comments · Fixed by #700
Assignees
Labels
Approved The issue is approved

Comments

@jhoek
Copy link

jhoek commented Oct 24, 2023

🕮 Codeunit 1483 XmlWriter lacks some overloads for WriteElementString()

The .NET framework provides a number of overloads for XmlWriter.WriteElementString() that accept a prefix/namespace. WriteStartElement in codeunit 1483 has an overload with Prefix and Namespace, but WriteElementString() does not. I would like to add two overloads for WriteElementString() in codeunit 1483, analogous to the .NET interface.

🔧 To Reproduce
Compare codeunit 1483 with https://learn.microsoft.com/en-us/dotnet/api/system.xml.xmlwriter.writeelementstring?view=net-7.0.

📣 Expected behavior
Overloads available to .NET developers also available to AL developers.

@pri-kise
Copy link
Contributor

@jhoek just to make sure, you want to add this new methods (including tests) by yourself?

@jhoek
Copy link
Author

jhoek commented Oct 25, 2023

Sure! Have not looked at existing tests yet, but should not be that hard.

@jhoek
Copy link
Author

jhoek commented Nov 8, 2023

[@pri-kise Am I right in my assumption that I cannot submit a PR before I get the go-ahead from you guys, and therefore should probably hold off development until your feedback?]

@pri-kise
Copy link
Contributor

pri-kise commented Nov 8, 2023

@jhoek that's currently the official process.

I'll ping @JesperSchulz to approve this issue, since this is a no-brainer and any similiar issue got approved in the past.
That's definetly a no-brainer.

Therefore I suggest the following:

  1. Recreate the issue on the new repository for the System Application Development, that was officially released last week at Directions EMEA 2023. https://github.com/microsoft/BCApps/issues
    I'd use the Bug-Template, since this extensibility bug in my opinion.
  2. Then prepare the Pull Request and wait for the official approval of the issue.
  3. Create a PR on the new repository for the system application.
  4. Prepare yourself for comments on your PR :)

@JesperSchulz JesperSchulz transferred this issue from microsoft/ALAppExtensions Nov 16, 2023
@JesperSchulz
Copy link
Contributor

Issue moved to the right place 😊 And I'll approve this issue. Create your PR whenever you're ready!

@JesperSchulz JesperSchulz added the Approved The issue is approved label Nov 16, 2023
@Drakonian
Copy link
Contributor

Since this is still here, can I create a PR?

@JesperSchulz

@JesperSchulz
Copy link
Contributor

You most certainly can!

Drakonian added a commit to Drakonian/BCApps that referenced this issue Mar 7, 2024
JesperSchulz pushed a commit that referenced this issue Mar 7, 2024
<!-- Thank you for submitting a Pull Request. If you're new to
contributing to BCApps please read our pull request guideline below
* https://github.com/microsoft/BCApps/Contributing.md
-->
#### Summary <!-- Provide a general summary of your changes -->
Additional overloads for WriteEelementString method.
Update tests

#### Work Item(s) <!-- Add the issue number here after the #. The issue
needs to be open and approved. Submitting PRs with no linked issues or
unapproved issues is highly discouraged. -->
Fixes #346 



Fixes
[AB#505053](https://dynamicssmb2.visualstudio.com/1fcb79e7-ab07-432a-a3c6-6cf5a88ba4a5/_workitems/edit/505053)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved The issue is approved
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants