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

Determine optional action parameters from CSN's notNull #332

Merged
merged 1 commit into from
Sep 18, 2024
Merged

Conversation

chgeo
Copy link
Member

@chgeo chgeo commented Sep 18, 2024

Replace the use of @Core.OptionalParameter for optional parameters (which was an intermediate solution anyways) by evaluating CSN's notNull property. This is in line with the decision in the CDS spec meeting and with what RFC importer now generates.

Note that this formally inverses the default for optional parameters:

  • They are all optional by default, which is how the runtime treats it.
  • Mandatory parameters now have to be marked with notNull (or not null in CDL).

Replace the use of `@Core.OptionalParameter` for optional parameters
(which was an intermediate solution anyways) by evaluating CSN's `notNull`
property.  This is in line with the decision in the CDS spec meeting and
with what RFC importer now generates.

Note that this formally inverses the default for optional parameters:
- They are all optional by default, which is how the runtime treats it.
- Mandatory parameters now have to be marked with `notNull`
  (or `not null` in CDL).
@chgeo chgeo requested a review from daogrady September 18, 2024 08:26
@chgeo chgeo changed the title Determine optional parameters from CSN's notNull Determine optional action parameters from CSN's notNull Sep 18, 2024
@daogrady
Copy link
Contributor

related: #315

@daogrady
Copy link
Contributor

daogrady commented Sep 18, 2024

by evaluating CSN's notNull property.

Has this property been present all along or does it require a certain version of @sap/cds, the compiler, or any other related lib?

@chgeo
Copy link
Member Author

chgeo commented Sep 18, 2024

Has this property been present all along or does it require a certain version of @sap/cds, the compiler, or any other related lib?

Has been there for a long time.

Copy link
Contributor

@daogrady daogrady left a comment

Choose a reason for hiding this comment

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

thanks!

@chgeo chgeo merged commit e02eb90 into main Sep 18, 2024
9 checks passed
@chgeo chgeo deleted the opt-params branch September 18, 2024 11:19
chgeo added a commit that referenced this pull request Oct 16, 2024
`array of` needs to be supported as well:

```cds
function check(
    param_2: Array of String not null
) returns ...
```

Explaining comment is
[here](#332 (comment)).

---------

Co-authored-by: Christian Georgi <[email protected]>
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