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 JellyFish Color Scheme #9004

Merged
merged 2 commits into from
Jan 9, 2025
Merged

add JellyFish Color Scheme #9004

merged 2 commits into from
Jan 9, 2025

Conversation

anubhavkrishna1
Copy link
Contributor

  • 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.
  • My package doesn't add context menu entries. *
  • My package doesn't add key bindings. **
  • Any commands are available via the command palette.
  • Preferences and keybindings (if any) are listed in the menu and the command palette, and open in split view.
  • If my package is a syntax it doesn't also add a color scheme. ***
  • If my package is a syntax it is named after the language it supports (without suffixes like "syntax" or "highlighting").
  • I use .gitattributes to exclude files from the package: images, test files, sublime-project/workspace.

My package is Dark color theme for sublime text 4.

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: SUCCESS

Repo link: JellyFish Color Theme

Packages added:
  - JellyFish Color Theme

Processing package "JellyFish Color Theme"
  - All checks passed

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: SUCCESS

Repo link: JellyFish Color Theme

Packages added:
  - JellyFish Color Theme

Processing package "JellyFish Color Theme"
  - All checks passed

@braver
Copy link
Collaborator

braver commented Nov 22, 2024

Couple of minor points:

  • It's, Sublime parlance, a "color scheme" and not a "theme", so also not a "color theme".
  • You seem to be missing some globals. I wonder if, when a user turns on "guides" whether they get a correct color for instance.
  • The color scheme itself could benefit from using variables for recurring colours.
  • Please consider excluding the preview images from the package using .gitattributes, saves needless bytes in transit and storage.

@anubhavkrishna1
Copy link
Contributor Author

@braver is this anubhavkrishna1/sublime-jellyfish@d8d45b6 good ?

@anubhavkrishna1 anubhavkrishna1 changed the title add JellyFish Color Theme add JellyFish Color Scheme Nov 23, 2024
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: SUCCESS

Repo link: JellyFish Color Scheme

Packages added:
  - JellyFish Color Scheme

Processing package "JellyFish Color Scheme"
  - All checks passed

@braver
Copy link
Collaborator

braver commented Nov 23, 2024

Yeah that should work. What’s left is to go through the readme etc to correct your package’s name.

@anubhavkrishna1
Copy link
Contributor Author

OK

@anubhavkrishna1
Copy link
Contributor Author

@braver I have corrected package name in my repo.

@anubhavkrishna1
Copy link
Contributor Author

@braver can you now merge this pull request ?

@braver
Copy link
Collaborator

braver commented Jan 1, 2025

Just a few minor things:

  • you've made a number of typos in your readme and repo description, where it says "sheme" instead of "scheme"
  • please rename JellyFish Theme.sublime-color-scheme to for instance simply JellyFish.sublime-color-scheme.

@braver braver added the mergeable The PR is in a mergeable state but awaiting some final comments/acknowledgement from either side label Jan 1, 2025
@anubhavkrishna1
Copy link
Contributor Author

@braver done

@braver braver merged commit be35d5f into wbond:master Jan 9, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feedback provided mergeable The PR is in a mergeable state but awaiting some final comments/acknowledgement from either side
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants