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 ryanolsonx/sublimetext-zenburn-theme #8319

Merged
merged 1 commit into from
Aug 6, 2021
Merged

Add ryanolsonx/sublimetext-zenburn-theme #8319

merged 1 commit into from
Aug 6, 2021

Conversation

ryanolsonx
Copy link
Contributor

@ryanolsonx ryanolsonx commented Jul 18, 2021

Ported the zenburn color scheme to sublime text. I have a VS Code version and would love to provide this in package control for sublime text users. This is vastly different than the existing zenburn theme. My theme is a lot closer to the Vim color scheme (that this was ported from).

@braver
Copy link
Collaborator

braver commented Jul 20, 2021

The existing package is actively maintained, why not open a PR there?

@braver braver added duplicate This package has already been submitted/accepted in a different pull request feedback provided labels Jul 20, 2021
@ryanolsonx
Copy link
Contributor Author

Okay - I'll work with that package author to see if I can incorporate my changes into that repo. If not, would we be able to get this in as a separate package, @braver?

@braver
Copy link
Collaborator

braver commented Jul 20, 2021

Sure, there is nothing fundamentally against multiple versions of the same idea, but all around it’s better if we can work together and present users with a single really good version.

@braver
Copy link
Collaborator

braver commented Jul 20, 2021

I’ll close this for now, just ping me to let me know if you worked it out or if we need to look at adding your version as well.

@braver braver closed this Jul 20, 2021
@ryanolsonx
Copy link
Contributor Author

@braver So far no response from @colinta after 3 days. I created an issue colinta/zenburn#8 and a PR colinta/zenburn#9. If there's no response by Jul 27th (after 1 week), would you mind re-opening this?

@braver
Copy link
Collaborator

braver commented Jul 23, 2021

We usually keep a 2 week timeout period for these things. @colinta seems to regularly check in in GitHub, so let's give him a little time.

@rwols
Copy link
Contributor

rwols commented Jul 23, 2021

Three days is very short for people doing this in their spare free time ;)

@ryanolsonx
Copy link
Contributor Author

@rwols I was thinking 1 week would be reasonable -- not 3 days.

I'm happy to wait 2 weeks 😃

@ryanolsonx
Copy link
Contributor Author

@braver Do you mind re-opening this?

@braver braver reopened this Aug 5, 2021
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: ERROR

Repo link: Zenburn Color Scheme
Results help

Packages added:
  - Zenburn Color Scheme

Processing package "Zenburn Color Scheme"
  - ERROR: The package contains 1 Python file(s), but none of them are in the package root and no build system is specified
  - 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 Aug 5, 2021

Alright, let's do this.

  • You have quite a few files that aren't functional to the package. You can exclude them from the package using .gitattributes and export-ingore, saving some storage bits and CO2 ;)
  • About the name, I think it should signal this is the new and improved Zenburn. "Zenburn New", "Zenburn Improved", something like that?
  • And have a look at the feedback from the bot.

@braver braver removed the duplicate This package has already been submitted/accepted in a different pull request label Aug 5, 2021
@ryanolsonx
Copy link
Contributor Author

@braver Alright - so I've fixed up a lot of the issues.

  • I removed the "demo" directory that wasn't really needed.
  • It has a license file now.

Let's talk about the name. I don't feel like the existing Zenburn color theme is really "Zenburn." Mine is a more accurate port of what Zenburn is all about and really matches up with what was originally created for Vim. With that said, would we be able to keep my proposed name? I feel like it would be misleading to users for them to see Zenburn Improved or something like that when I haven't changed the nature of the Zenburn theme itself (and as it's not a variant of Zenburn).

Thoughts?

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: Zenburn Color Scheme

Packages added:
  - Zenburn Color Scheme

Processing package "Zenburn Color Scheme"
  - All checks passed

@braver
Copy link
Collaborator

braver commented Aug 6, 2021

Well you still have a lot of screenshots in the package, I would encourage you to use the .gitattributes for that. I also kinda doubt that they're useful to new users.

I agree with your reasoning about the name though, let's keep it as it is.

@ryanolsonx
Copy link
Contributor Author

Awesome.

I'll remove those screenshots. It should be much better now.

@braver braver merged commit 4015b5c into wbond:master Aug 6, 2021
@ryanolsonx
Copy link
Contributor Author

Thanks a lot @braver!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants