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

CCSMB-9: Disabling Automatic Updating and Dependency Installation in Lua Programs #25

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

tomodachi94
Copy link
Member

The title says it all. (I hope?)

See the Rationale section for rationale.

@tomodachi94 tomodachi94 added classification: proposal Introduction of a new proposal. status: work in progress Needs more work before it can be merged. labels Aug 26, 2023
@tomodachi94 tomodachi94 marked this pull request as ready for review August 26, 2023 03:48
Standards/CCSMB-9.md Outdated Show resolved Hide resolved
Standards/CCSMB-9.md Outdated Show resolved Hide resolved
Copy link
Member

@scmcgowen scmcgowen left a comment

Choose a reason for hiding this comment

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

Agree with DVD and noted typographical error

Standards/CCSMB-9.md Outdated Show resolved Hide resolved
Standards/CCSMB-9.md Outdated Show resolved Hide resolved
@tomodachi94 tomodachi94 added status: response from author needed A proposal needs a response or modification by the author. status: stale An unmerged pull request has sat for a month with no comments. labels Mar 24, 2024
@tomodachi94 tomodachi94 added status: reviews needed A proposal needs reviews. and removed status: response from author needed A proposal needs a response or modification by the author. status: stale An unmerged pull request has sat for a month with no comments. labels Mar 24, 2024
Copy link

@DVD-DAVIDE DVD-DAVIDE left a comment

Choose a reason for hiding this comment

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

There are some inconsistencies in the document.

Standards/CCSMB-9.md Outdated Show resolved Hide resolved
Standards/CCSMB-9.md Outdated Show resolved Hide resolved
Standards/CCSMB-9.md Outdated Show resolved Hide resolved
Standards/CCSMB-9.md Outdated Show resolved Hide resolved
Copy link

@DVD-DAVIDE DVD-DAVIDE left a comment

Choose a reason for hiding this comment

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

Seems fine to me.

@tomodachi94 tomodachi94 removed the status: work in progress Needs more work before it can be merged. label Mar 25, 2024
# CCSMB 9: Disabling Automatic Updating and Dependency Installation in Lua Programs

*Author: Tomodachi94 <@tomodachi94>*

Copy link
Member

Choose a reason for hiding this comment

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

I think we should add the text referring to RFC 2119 at the top here if we're gonna use the keywords


### Global variable

Compliant programs MUST allow disabling autoupdating through the `CCSMB_ENABLE_AUTOUPDATING` global variable. If the global is set to `false`, autoupdating MUST be disabled. If the variable is unset, compliant programs MAY choose to enable or disable autoupdating for itself.
Copy link
Member

Choose a reason for hiding this comment

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

And what if the global variable is true? I think this should also be defined

Programs implementing autoinstallation of dependencies MUST implement all options listed below in order to be considered compliant.

### `settings` API setting
Compliant programs MUST allow disabling dependency installation through the `ccsmb.dependencyInstallation.enable` setting. If the option is set to `true`, autoinstallation of dependencies MUST be disabled. If the option is unset, compliant programs MAY choose to enable or disable dependency installation for itself.
Copy link
Member

Choose a reason for hiding this comment

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

And if its false?

I may be annoying about this but I think all options should be clearly defined


### Global variable

Compliant programs MUST allow disabling autoinstallation of dependencies through the `CCSMB_ENABLE_DEPENDENCY_INSTALLATION` global variable. If the global is set to `false`, autoupdating MUST be disabled. If the option is unset, compliant programs MAY choose to enable or disable dependency installation for itself.
Copy link
Member

Choose a reason for hiding this comment

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

And here too, if its set to true the behaviour should be defined.

Programs implementing an autoupdater MUST implement all options listed below in order to be considered compliant.

### `settings` API setting
Compliant programs MUST allow disabling autoupdating through the `ccsmb.autoupdate.enable` setting. If the option is set to `false`, autoupdating MUST be disabled. If the option is unset, compliant programs MAY choose to enable or disable autoupdating for itself.
Copy link
Member

Choose a reason for hiding this comment

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

Here too we should probably define what happens if its true

Programs implementing autoinstallation of dependencies MUST implement all options listed below in order to be considered compliant.

### `settings` API setting
Compliant programs MUST allow disabling dependency installation through the `ccsmb.dependencyInstallation.enable` setting. If the option is set to `true`, autoinstallation of dependencies MUST be disabled. If the option is unset, compliant programs MAY choose to enable or disable dependency installation for itself.
Copy link
Member

Choose a reason for hiding this comment

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

If its set to true, it should be enabled, not disabled, its way more clear

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
classification: proposal Introduction of a new proposal. status: reviews needed A proposal needs reviews.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants