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

docs: Update input schema specs #1307

Merged
merged 5 commits into from
Nov 28, 2024
Merged

docs: Update input schema specs #1307

merged 5 commits into from
Nov 28, 2024

Conversation

jancurn
Copy link
Member

@jancurn jancurn commented Nov 27, 2024

No description provided.

@jancurn jancurn requested a review from TC-MO as a code owner November 27, 2024 14:51
Comment on lines 21 to 22
To define an input schema for an Actor, set `input` field in the `.actor/actor.json` file to an input schema object as described below, or path to a JSON file containing the input schema.
For backwards compability, if the `input` field is omitted, the system looks for an `INPUT_SCHEMA.json` file in the `.actor` directory or for an `INPUT_SCHEMA.json` file in the Actor's root directory - but do not depend on this behavior as it might be removed in the future.
Copy link
Contributor

@TC-MO TC-MO Nov 27, 2024

Choose a reason for hiding this comment

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

Suggested change
To define an input schema for an Actor, set `input` field in the `.actor/actor.json` file to an input schema object as described below, or path to a JSON file containing the input schema.
For backwards compability, if the `input` field is omitted, the system looks for an `INPUT_SCHEMA.json` file in the `.actor` directory or for an `INPUT_SCHEMA.json` file in the Actor's root directory - but do not depend on this behavior as it might be removed in the future.
To define an input schema for an Actor, set `input` field in the `.actor/actor.json` file to an input schema object as described below, or path to a JSON file containing the input schema.
For backwards compatibility, if the `input` field is omitted, the system checks these locations:
1. `INPUT_SCHEMA.json` in the `.actor` directory
2. `INPUT_SCHEMA.json` in the Actor's root directory

I would omit any information about what might or might not happen, if it does then we can update the documentation with a note. If we want to steer users in one direction we can add that some info about preferred or recommended way of doing things

Copy link
Member Author

Choose a reason for hiding this comment

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

IMO it's quite normal in docs to say that some behavior might be deprecetated in the future, so that people don't use it - this is exactly the case

Copy link
Contributor

Choose a reason for hiding this comment

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

In my experience with docuemntation, I've never seen a message with possibility of depreacation. It was always with specific end-of-life dates.

If we want to guide users towards a preferred implementation I would advise to explicitly mention which way is recommended.

Copy link
Member Author

Choose a reason for hiding this comment

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

Really? I see it all the time in docs that some feature is deprecated and might be removed in the future. See https://www.google.com/search?q=deprecated+and+might+be+removed+in+the+future

Copy link
Contributor

Choose a reason for hiding this comment

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

But none of the results are from official documentation but rather community resources.

Furthermore
deprecated & might be removed in the future =/= might be removed in the future,
if the intention is that these two ways of defining input schema are deprecated and & will be removed in the future, then let's mark them explicitly as deprecated and then mentioning that they might be removed in the future is a-ok

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh right, fair enough, I'll mention it's deprecated

@jancurn jancurn requested a review from TC-MO November 28, 2024 09:26
@jancurn jancurn merged commit 868b576 into master Nov 28, 2024
7 checks passed
@jancurn jancurn deleted the jancurn-patch-1 branch November 28, 2024 09:36
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.

2 participants