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

Optional scripts section in libraries' config #11555

Closed
wants to merge 4 commits into from

Conversation

hubertp
Copy link
Collaborator

@hubertp hubertp commented Nov 14, 2024

Pull Request Description

This change adds a scripts/hooks section to config that allows to specify a list of available scripts for the given library. Scripts are defined in a free-form, meaning that any kind of value can be provided as a value and is parsed as a string.

For example,

scripts:
  refresh:
    - Standard.Base.HTTP.Caches.refresh

Part of #11485.

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • The documentation has been updated, if necessary.
  • Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
  • All code follows the
    Scala,
    Java,
    TypeScript,
    and
    Rust
    style guides. In case you are using a language not listed above, follow the Rust style guide.
  • Unit tests have been written where possible.
  • If meaningful changes were made to logic or tests affecting Enso Cloud integration in the libraries,
    or the Snowflake database integration, a run of the Extra Tests has been scheduled.
    • If applicable, it is suggested to paste a link to a successful run of the Extra Tests.

This change adds a scripts/hooks section to config that allows to
specify a list of available scripts for the given library. Scripts are
defined in a free-form, meaning that any kind of value can be provided
as a value and is parsed as a string.

For example,
```
scripts:
  refresh:
    - Standard.Base.HTTP.Caches.refresh
```
@JaroslavTulach
Copy link
Member

There is a checklist in this PR (as well as in any other PR) asking for few checks including:

  • The documentation has been updated, if necessary.

I guess this time, when we design a new API for library writers, we should start by writing the documentation, updating changelog. Etc.

adds a scripts/hooks section to config that allows to specify a list of available scripts for the given library. Scripts are defined in a free-form, meaning that any kind of value can be provided as a value and is parsed as a string.

I'd like to see list of use-cases justifying the API decisions first. Really, "freeform"? Why? What are the scripts good for? How can one use them? When they are called?

@hubertp
Copy link
Collaborator Author

hubertp commented Nov 14, 2024

I'd like to see list of use-cases justifying the API decisions first.

This PR unblocks further work that we have discussed internally for the purpose of #11485. Thanks for your feedback.

@@ -599,7 +600,8 @@ private void createNew(
authors,
nil(),
"",
Option$.MODULE$.empty());
Option$.MODULE$.empty(),
List$.MODULE$.empty());
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can use nil() helper method (see two lines above)

lib/scala/pkg/src/main/scala/org/enso/pkg/Script.scala Outdated Show resolved Hide resolved
@4e6
Copy link
Contributor

4e6 commented Nov 14, 2024

Please update ConfigSpec

@hubertp hubertp added the CI: No changelog needed Do not require a changelog entry for this PR. label Nov 14, 2024
@hubertp hubertp requested a review from 4e6 November 14, 2024 17:10
@hubertp
Copy link
Collaborator Author

hubertp commented Nov 18, 2024

Currently on hold to test an alternative solution #11485 (comment)

@hubertp
Copy link
Collaborator Author

hubertp commented Nov 19, 2024

Made obsolete via #11577. We could still revive it, if needed.

@hubertp hubertp closed this Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: No changelog needed Do not require a changelog entry for this PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants