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.

Sign up for GitHub

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

Jump to bottom

Compile Fix #520

Merged
merged 10 commits into from Dec 14, 2023
Merged

Compile Fix #520

merged 10 commits into from Dec 14, 2023

Conversation

DustinGrau-PSC
Copy link
Contributor

Description

Adds additional error handling for creating the callback class for 11.3+ installations.

Fixes # (issue)

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • [-] New feature (non-breaking change which adds functionality)
  • [-] Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • [-] This change requires a documentation update

Checklist:

  • My branch is started from the latest commit in main
  • [-] My commit history is clean
  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • [-] I have made corresponding changes to the documentation
  • [-] My changes generate no new warnings
  • [-] I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

gquerret
gquerret reviewed Dec 14, 2023
View reviewed changes
src/progress/pct/compile.p Outdated
@@ -212,15 +212,33 @@ END PROCEDURE.
PROCEDURE initModule:
ASSIGN lIgnoredIncludes = (LENGTH(cignoredIncludes) > 0).

IF (callbackClass > "") AND NOT bAboveEq113 THEN
IF (callbackClass > "") EQ TRUE AND NOT bAboveEq113 THEN
Copy link
Contributor

Choose a reason for hiding this comment

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

@DustinGrau-PSC What's that ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That should handle the case where callbackClass may somehow be the unknown value ? as well as an empty string. It ensures that the value is truly a string.

Copy link
Contributor

Choose a reason for hiding this comment

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

I've tried with multiple values (?, empty string, ...) and the eq true is not necessary

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed. It was more of a suggestion/habit when I use the > "" syntax for string checks.

@gquerret gquerret merged commit 6056243 into Riverside-Software:main Dec 14, 2023
1 check passed
@DustinGrau-PSC DustinGrau-PSC deleted the compilefix branch December 14, 2023 21:46
@DustinGrau-PSC
Copy link
Contributor Author

Merci! Will there be a new release version created, or does that only occur when there are significant updates?

@gquerret
Copy link
Contributor

v227 was released last week, so no, there won't be a new release soon. You can always download builds here: https://ci.rssw.eu/job/PCT/job/main/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gquerret gquerret gquerret left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@DustinGrau-PSC @gquerret