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

Add Hadar Theme to h.json #8750

Closed
wants to merge 10 commits into from
Closed

Conversation

cristianvasquezc
Copy link

@cristianvasquezc cristianvasquezc commented May 14, 2023

  • I'm the package's author and/or maintainer.
  • I have have read the docs.
  • I have tagged a release with a semver version number.
  • My package repo has a description and a README describing what it's for and how to use it.
  • Any commands are available via the command palette.
  • If my package is a syntax it is named after the language it supports (without suffixes like "syntax" or "highlighting").

There are no packages like it in Package Control.

Copy link
Collaborator

@packagecontrol-bot packagecontrol-bot left a comment

Choose a reason for hiding this comment

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

Automated testing result: WARNING

Repo link: Hadar Theme
Results help

Packages added:
  - Hadar Theme

Processing package "Hadar Theme"
  - WARNING: The package does not contain a top-level LICENSE file. A license helps users to contribute to the package.

Copy link
Collaborator

@packagecontrol-bot packagecontrol-bot left a comment

Choose a reason for hiding this comment

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

Automated testing result: WARNING

Repo link: Hadar Theme
Results help

Packages added:
  - Hadar Theme

Processing package "Hadar Theme"
  - WARNING: The package does not contain a top-level LICENSE file. A license helps users to contribute to the package.

@braver
Copy link
Collaborator

braver commented Jun 10, 2023

Your theme seems to have some settings. Shouldn't they be exposed to the end user?

Copy link
Collaborator

@packagecontrol-bot packagecontrol-bot left a comment

Choose a reason for hiding this comment

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

Automated testing result: WARNING

Repo link: Hadar Theme
Results help

Packages added:
  - Hadar Theme

Processing package "Hadar Theme"
  - WARNING: The package does not contain a top-level LICENSE file. A license helps users to contribute to the package.

Copy link
Collaborator

@packagecontrol-bot packagecontrol-bot left a comment

Choose a reason for hiding this comment

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

Automated testing result: WARNING

Repo link: Hadar Theme
Results help

Packages added:
  - Hadar Theme

Processing package "Hadar Theme"
  - WARNING: The package does not contain a top-level LICENSE file. A license helps users to contribute to the package.

Copy link
Collaborator

@packagecontrol-bot packagecontrol-bot left a comment

Choose a reason for hiding this comment

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

Automated testing result: WARNING

Repo link: Hadar Theme
Results help

Packages added:
  - Hadar Theme

Processing package "Hadar Theme"
  - WARNING: The package does not contain a top-level LICENSE file. A license helps users to contribute to the package.

@braver
Copy link
Collaborator

braver commented Jun 21, 2023

The bot feedback is fine, you don't need a license file. Please address my earlier feedback about the settings.

Copy link
Collaborator

@packagecontrol-bot packagecontrol-bot left a comment

Choose a reason for hiding this comment

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

Automated testing result: WARNING

Repo link: Hadar Theme
Results help

Packages added:
  - Hadar Theme

Processing package "Hadar Theme"
  - WARNING: The package does not contain a top-level LICENSE file. A license helps users to contribute to the package.

Copy link
Collaborator

@packagecontrol-bot packagecontrol-bot left a comment

Choose a reason for hiding this comment

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

Automated testing result: WARNING

Repo link: Hadar Theme
Results help

Packages added:
  - Hadar Theme

Processing package "Hadar Theme"
  - WARNING: The package does not contain a top-level LICENSE file. A license helps users to contribute to the package.

@braver braver added the stale The pull request needs to be updated but has not been within the recent past (2 weeks) label Jul 8, 2023
@braver
Copy link
Collaborator

braver commented Jul 20, 2023

Your theme seems to have some settings. Shouldn't they be exposed to the end user?

☝🏻

@cristianvasquezc
Copy link
Author

Y como hago eso de la configuración? 🤔

@braver
Copy link
Collaborator

braver commented Jul 21, 2023

You would usually have a menu entry (and a command palette entry) that allows a user to open and edit the settings.

@braver braver removed the stale The pull request needs to be updated but has not been within the recent past (2 weeks) label Jul 23, 2023
@braver
Copy link
Collaborator

braver commented Aug 25, 2023

I'm going to close this as stale, please respond (hopefully in English, sorry about that) if you want to continue.

@braver braver closed this Aug 25, 2023
@braver braver added the timeout A pull request needed changes but was not updated in time (2 weeks after becoming stale) label Aug 25, 2023
@jwortmann
Copy link
Contributor

You would usually have a menu entry (and a command palette entry) that allows a user to open and edit the settings.

Those menu and command palette entries are already included in Sublime Text; the menu entry is under Preferences > Settings (might be different on macOS (?)) and the command palette entry is Preferences: Settings.

Note that unlike plugins, themes cannot define their own settings files, instead settings are always read from the genereal Preferences.sublime-settings file.

In theory, a theme could add another entry to open a predefined Preferences.sublime-settings file on the lefthand side, and the users setting on the righthand side, but then the built-in settings which are expected to be implemented by themes, wouldn't be visible. And I doubt that there are many themes which do it this way. Instead, I would just describe the settings in the Readme page.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feedback provided timeout A pull request needed changes but was not updated in time (2 weeks after becoming stale)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants