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

Bugfix/ls24004056/packed coverage #618

Merged
merged 13 commits into from
Sep 30, 2024

Conversation

dom-apuliasoft
Copy link
Collaborator

Description

This PR does not add or change any RPG language functionality

From a technical standpoint changes are:

Related to:

Checklist:

  • If this feature involves RPGLE fixes or improvements, they are well-described in the summary.
  • There are tests for this feature.
  • RPGLE code used for tests is easily understandable and includes comments that clarify the purpose of this feature.
  • The code follows Kotlin conventions (run ./gradlew ktlintCheck).
  • The code passes all tests (run ./gradlew check).
  • Relevant documentation is included in the docs directory.

Copy link
Collaborator

@lanarimarco lanarimarco left a comment

Choose a reason for hiding this comment

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

I appreciate implementations where you cut than adding to it! 😊

In addition to my comments on specific files, it seems there's no test that follows this structure:

  • Declare a DS1 with StringType, NumberType zoned, and NumberType packed.
  • Declare S1, a standalone StringType similar to DS1.
  • Declare DS2, structured like DS1.
  • Assign DS1 to S1 and then assign S1 to DS2.
  • Assert that each field of DS1 is equal to the corresponding field in DS2.

Copy link
Collaborator

@lanarimarco lanarimarco left a comment

Choose a reason for hiding this comment

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

I have a doubt about a test

@dom-apuliasoft
Copy link
Collaborator Author

As I understand my changes to MUTE13_41 were not appropriate I rolled it back to the original version @benetti-smeup gave me and only added the MUTE assertions in it. Let me know if it ok now and else-wise what needs to be changed.

@lanarimarco lanarimarco merged commit 9c1a861 into develop Sep 30, 2024
1 check passed
@lanarimarco lanarimarco deleted the bugfix/LS24004056/packed-coverage branch September 30, 2024 10:46
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