Include the optional LongName attribute in the XML encoded output. #308

laxmi-lal-menaria · 2022-06-10T17:26:41Z

Added a property which include the Long Name in Encoded XML

laxmi-lal-menaria · 2022-06-22T10:40:43Z

How it can merge to master?

milkshakeuk · 2022-06-23T08:12:21Z

@lmenaria I haven't had a chance to fully review the changes yet but I have noticed there are no unit tests, Can you add some unit test please, I'll hopefully get a chance to do a proper review soon.

milkshakeuk

Also @lmenaria not sure if you saw but there are a lot of static code analysis warnings which need fixing.

src/NHapi.Base/Parser/XMLParser.cs

milkshakeuk · 2022-07-06T13:33:12Z

@lmenaria have you seen this yet?

laxmi-lal-menaria · 2022-07-06T17:58:49Z

yes, but how we can use parserOptions in those both methods,we need to pass or create private variable for it and assign?

what do you think?

milkshakeuk · 2022-07-06T18:02:02Z

@lmenaria I would say pass it down, have a look at how the PipeParser uses it.

github-actions · 2022-07-09T13:57:54Z

Unit Test Results

1 372 tests 1 284 ✔️ 4s ⏱️
      3 suites     88 💤
      3 files       0 ❌

Results for commit 6054823.

♻️ This comment has been updated with latest results.

src/NHapi.Base/Parser/ParserOptions.cs

milkshakeuk · 2022-07-11T08:12:46Z

@lmenaria a bunch of code style issues need fixing, see automated comments in PR.

Also I feel some of the xml doc comments need updating, from the scan I did anyway.

Also still no unit tests for these changes.

milkshakeuk · 2022-07-15T08:15:01Z

@lmenaria still a bunch of static analysis warnings which need fixing:

laxmi-lal-menaria · 2022-07-19T17:16:16Z

Fixed this warning.

milkshakeuk · 2022-07-19T18:53:59Z

@lmenaria hi, thanks for that, however that wasn't the only static code analysis warning, if you look through the Files Changed you will see more easy to fix warnings.

Also there are still no unit tests to cover the new functionality, could you please add the appropriate unit tests.

Thank you.

laxmi-lal-menaria · 2022-07-26T10:20:05Z

Anything more need to do?

milkshakeuk · 2022-07-26T11:43:36Z

Anything more need to do?

@laxmi-lal-menaria yes please, can you add appropriate unit tests for the new functionality.

tests/NHapi.NUnit/Parser/PipeParserTests.cs

milkshakeuk · 2022-08-01T14:27:26Z

@laxmi-lal-menaria did you notice the failed build?

milkshakeuk · 2022-08-04T10:45:53Z

@laxmi-lal-menaria failed again.

milkshakeuk · 2022-08-05T11:26:44Z

@laxmi-lal-menaria looks like it failed again https://github.com/nHapiNET/nHapi/runs/7677435616?check_suite_focus=true

milkshakeuk · 2022-08-08T14:44:12Z

@laxmi-lal-menaria CI is still failing https://github.com/nHapiNET/nHapi/runs/7695823531?check_suite_focus=true

laxmi-lal-menaria · 2022-09-01T17:00:17Z

Testcases passed on Windows but not at ubuntu, I don't have any setup to fix that, if anyone can do it will be great help.

milkshakeuk · 2022-10-14T18:56:18Z

@laxmi-lal-menaria I fixed the issues with your PR, not sure if we need more/better tests (better as in obviouse what the test is doing).

milkshakeuk · 2022-10-21T10:11:09Z

@AMCN41R could you look at this? does it need anything more?

* Port `hapi` version of `XMLParser` and `DefaultXMLParser` - `nHapi` versions of these were years behind `hapi`. * Keep old implementations as `LegacyXMLParser` and `LegacyDefaultXMLParser` for people or depend on old imperfect behaviour * Add Unit tests for changes * Fix some code styling warnings * Improve XML Documentation * Update some Nuget packages * Unblocks #308 * Closes some Dependabot pull requests

* Port hapi version of XMLParser and DefaultXMLParser - nHapi versions of these were years behind hapi. * Keep old implementations as LegacyXMLParser and LegacyDefaultXMLParser for people or depend on old imperfect behaviour * Add Unit tests for changes * Fix some code styling warnings * Improve XML Documentation * Update some Nuget packages * Unblocks #308 * Closes some Dependabot pull requests

Added a property which include the Long Name in Encoded XML

Make 'includeLongNameInEncodedXML' private. Make this field 'private' and encapsulate it in a 'public' property. Comments for ignoring error

Removed this initialization to '_includeLongNameInEncodedXML'

milkshakeuk · 2023-01-06T15:36:00Z

@laxmi-lal-menaria okay I have rebased this PR based on what is now in master (makes this PR much smaller).

@AMCN41R @PhantomGrazzler could you give a quick once over - it's only a small PR.

src/NHapi.Base/Parser/ParserOptions.cs

Moved IncludeLongNameInEncodedXml to ParserOptions, instead of public property.

Removed unused methods Added summary for new variable

change new position

Added Testcase for IncludeLongNameInEncodedXML. Test Method for Encode Test Method for Parser

Casting issue fix with new message

removed not supported property in every versions.

laxmi-lal-menaria mentioned this pull request Jun 13, 2022

Include the optional LongName attribute in the XML encoded output. #301

Closed

laxmi-lal-menaria closed this Jun 22, 2022

laxmi-lal-menaria reopened this Jun 22, 2022

milkshakeuk reviewed Jun 26, 2022

View reviewed changes

src/NHapi.Base/Parser/XMLParser.cs Outdated Show resolved Hide resolved

src/NHapi.Base/Parser/XMLParser.cs Outdated Show resolved Hide resolved

src/NHapi.Base/Parser/XMLParser.cs Outdated Show resolved Hide resolved

milkshakeuk reviewed Jul 11, 2022

View reviewed changes

src/NHapi.Base/Parser/ParserOptions.cs Outdated Show resolved Hide resolved

milkshakeuk requested changes Jul 28, 2022

View reviewed changes

tests/NHapi.NUnit/Parser/PipeParserTests.cs Outdated Show resolved Hide resolved

milkshakeuk self-requested a review July 29, 2022 10:10

milkshakeuk linked an issue Oct 13, 2022 that may be closed by this pull request

Include the optional LongName attribute in the XML encoded output. #301

Closed

milkshakeuk force-pushed the master branch from 570e8fc to 75e1fc8 Compare October 14, 2022 18:41

milkshakeuk force-pushed the master branch from 75e1fc8 to c6473d5 Compare October 21, 2022 08:58

milkshakeuk requested a review from AMCN41R October 21, 2022 10:11

laxmi-lal-menaria added 3 commits January 6, 2023 13:05

Update XMLParser.cs

46d1c5f

Added a property which include the Long Name in Encoded XML

Update XMLParser.cs

eaf923d

Make 'includeLongNameInEncodedXML' private. Make this field 'private' and encapsulate it in a 'public' property. Comments for ignoring error

Update XMLParser.cs

ccbe2aa

Removed this initialization to '_includeLongNameInEncodedXML'

milkshakeuk force-pushed the master branch from c6473d5 to 5dfdd31 Compare January 6, 2023 15:33

milkshakeuk requested a review from AMCN41R January 6, 2023 15:36

milkshakeuk approved these changes Jan 6, 2023

View reviewed changes

AMCN41R approved these changes Jan 6, 2023

View reviewed changes

src/NHapi.Base/Parser/ParserOptions.cs Outdated Show resolved Hide resolved

laxmi-lal-menaria added 3 commits January 6, 2023 15:42

Moved IncludeLongNameInEncodedXml to ParserOptions

d49b73c

Moved IncludeLongNameInEncodedXml to ParserOptions, instead of public property.

Added Comments

e5604e5

Removed unused methods Added summary for new variable

change new position

b765173

change new position

milkshakeuk force-pushed the master branch from 5dfdd31 to 0b8e738 Compare January 6, 2023 15:44

laxmi-lal-menaria and others added 4 commits January 6, 2023 15:47

Added Testcase

fc93df4

Added Testcase for IncludeLongNameInEncodedXML. Test Method for Encode Test Method for Parser

Casting issue fix with new message

ee46677

Casting issue fix with new message

removed not supported property

8e0a1c1

removed not supported property in every versions.

Refactor test name to better describe the test.

6054823

milkshakeuk force-pushed the master branch from 0b8e738 to 6054823 Compare January 6, 2023 15:49

PhantomGrazzler approved these changes Jan 8, 2023

View reviewed changes

milkshakeuk merged commit c2d9c04 into nHapiNET:master Jan 8, 2023

milkshakeuk added this to the v3.2.0 milestone Jan 20, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Include the optional LongName attribute in the XML encoded output. #308

Include the optional LongName attribute in the XML encoded output. #308

laxmi-lal-menaria commented Jun 10, 2022

laxmi-lal-menaria commented Jun 22, 2022

milkshakeuk commented Jun 23, 2022

milkshakeuk left a comment

milkshakeuk commented Jul 6, 2022

laxmi-lal-menaria commented Jul 6, 2022

milkshakeuk commented Jul 6, 2022

github-actions bot commented Jul 9, 2022 •

edited

Loading

milkshakeuk commented Jul 11, 2022 •

edited

Loading

milkshakeuk commented Jul 15, 2022

laxmi-lal-menaria commented Jul 19, 2022

milkshakeuk commented Jul 19, 2022 •

edited

Loading

laxmi-lal-menaria commented Jul 26, 2022

milkshakeuk commented Jul 26, 2022

milkshakeuk commented Aug 1, 2022

milkshakeuk commented Aug 4, 2022

milkshakeuk commented Aug 5, 2022

milkshakeuk commented Aug 8, 2022

laxmi-lal-menaria commented Sep 1, 2022

milkshakeuk commented Oct 14, 2022

milkshakeuk commented Oct 21, 2022

milkshakeuk commented Jan 6, 2023

Include the optional LongName attribute in the XML encoded output. #308

Include the optional LongName attribute in the XML encoded output. #308

Conversation

laxmi-lal-menaria commented Jun 10, 2022

laxmi-lal-menaria commented Jun 22, 2022

milkshakeuk commented Jun 23, 2022

milkshakeuk left a comment

Choose a reason for hiding this comment

milkshakeuk commented Jul 6, 2022

laxmi-lal-menaria commented Jul 6, 2022

milkshakeuk commented Jul 6, 2022

github-actions bot commented Jul 9, 2022 • edited Loading

Unit Test Results

milkshakeuk commented Jul 11, 2022 • edited Loading

milkshakeuk commented Jul 15, 2022

laxmi-lal-menaria commented Jul 19, 2022

milkshakeuk commented Jul 19, 2022 • edited Loading

laxmi-lal-menaria commented Jul 26, 2022

milkshakeuk commented Jul 26, 2022

milkshakeuk commented Aug 1, 2022

milkshakeuk commented Aug 4, 2022

milkshakeuk commented Aug 5, 2022

milkshakeuk commented Aug 8, 2022

laxmi-lal-menaria commented Sep 1, 2022

milkshakeuk commented Oct 14, 2022

milkshakeuk commented Oct 21, 2022

milkshakeuk commented Jan 6, 2023

github-actions bot commented Jul 9, 2022 •

edited

Loading

milkshakeuk commented Jul 11, 2022 •

edited

Loading

milkshakeuk commented Jul 19, 2022 •

edited

Loading