search

aziz / PlainTasks

An opinionated todo-list plugin for Sublime Text editor (version 2 and 3)
MIT License
3.29k stars 286 forks source link

New theme and new close icon #385

Closed johntor closed 6 years ago

vovkkk commented 6 years ago

I won’t merge it, I hope reasons are obvious. But if you insist on the optional cancel bullet, feel free to send another pr.

johntor commented 6 years ago

Sorry to bother, I don't mind if you don't merge. What is the obvious? Please explain?

vovkkk commented 6 years ago

The reasons not to merge it:

  1. Screenshot was committed—repo is not the place for screenshots, because on every automatic update the whole package will be downloaded, we must keep the size of package reasonable. Also screenshot does not provide any functionality (unlike e.g. icon) which is another reason to keep it outside of repo.
  2. Two unrelated features were proposed in one pr, that’s inconvenient because it forces us to deal with it as with one thing although it is actual two things.
  3. You changed defaults values, obv. it is no-no; such changes should be discussed in the first place, but I guarantee that chances that we agree to change defaults are very low.
  4. From now on I won’t accept new colour schemes because we already have a good range to choose from, adding more will just increase the size of the package (see 1.). User can set any colour schemes from any packages including User folder.
johntor commented 6 years ago

Thank you very much for the reply! You are almost 100% right. The Screenshot I uploaded was just to show the theme because otherwise you would not bother to see it so I agree with you. The theme I think is very nice so I think maybe someone else can get it from my repo. I didn't know the policy about package size that I respect. Finally I would suggest you should include the new icon because the default sucks on windows. It was my first pull request so I admit it was a mistake proposing two changes in one.

Keep on the good job!!!