Skip to content

Conversation

@m-mohr
Copy link
Member

@m-mohr m-mohr commented Oct 12, 2023

See the README for details.

This PR includes the changes from PR #490.

@m-mohr m-mohr force-pushed the add-tests branch 3 times, most recently from f7affe8 to dafc6d9 Compare October 13, 2023 16:39
@soxofaan
Copy link
Member

soxofaan commented Jan 5, 2024

I have the impression a lot of changes in the diff come from PR #490
Is that intended?

@m-mohr
Copy link
Member Author

m-mohr commented Aug 7, 2025

Thank you @soxofaan. I've rebased and the diff looks significantly cleaner now.

Copy link
Member

@soxofaan soxofaan left a comment

Choose a reason for hiding this comment

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

This PR is too large to give it a full in-depth review, but here are some random notes.

I guess it's more important to get this merged as "good enough" so that implementations can start integrate it, than to reach perfection.

"arguments": {
"data": []
},
"returns": {"type": "nodata"}
Copy link
Member

Choose a reason for hiding this comment

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

FYI:

Suggested change
"returns": {"type": "nodata"}
"returns": false

per #494

Copy link
Member Author

@m-mohr m-mohr Aug 22, 2025

Choose a reason for hiding this comment

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

Yeah, but #550 has not been merged yet. We need to merge it first.

"arguments": {
"data": []
},
"returns": {"type": "nodata"}
Copy link
Member

Choose a reason for hiding this comment

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

FYI:

Suggested change
"returns": {"type": "nodata"}
"returns": true

per #494

Copy link
Member Author

Choose a reason for hiding this comment

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

Waiting for #550.

@m-mohr
Copy link
Member Author

m-mohr commented Aug 22, 2025

I merged in from draft, but the tests still need some updates to reflect the latest changes in there. Need to go through the changes and adapt the tests. And also add tests for text_find.

m-mohr and others added 4 commits September 8, 2025 14:38
Co-authored-by: Stefaan Lippens <soxofaan@users.noreply.github.com>
Co-authored-by: Stefaan Lippens <soxofaan@users.noreply.github.com>
@m-mohr m-mohr merged commit cc0eedc into draft Sep 8, 2025
2 checks passed
@m-mohr m-mohr deleted the add-tests branch September 8, 2025 12:58
@github-project-automation github-project-automation bot moved this from In Progress to Done in Next processes release Sep 8, 2025
@m-mohr m-mohr restored the add-tests branch September 8, 2025 12:58
@m-mohr
Copy link
Member Author

m-mohr commented Sep 8, 2025

Merged so that we can update following PRs such as #551 and #494. This has been reviewed multiple times and is part of the test suite already.

Note here, I'm keeping the branch alive for now as the test suite is referring to it, which we'll need to update.

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

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

OpenEO processes validation suite

3 participants