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 XdebugHelper #8825

Closed
wants to merge 1 commit into from
Closed

Add XdebugHelper #8825

wants to merge 1 commit into from

Conversation

maliayas
Copy link
Contributor

@maliayas maliayas commented Sep 30, 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.
  • 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.

My package is XdebugHelper

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

Packages added:
  - XdebugHelper

Processing package "XdebugHelper"
  - All checks passed

@braver
Copy link
Collaborator

braver commented Nov 19, 2023

Thanks for your submission. Sorry we kept it sitting there for a while, a package like this needs a bit more thinking about to review then say a color scheme :)

  • There are multiple Xdebug packages in package control, none of which are called "SublimeTextXdebug". The repo might be called that way but the package is named differently. It might help users if you link to the specific one on package control you're interfacing with.
  • As for the package itself the only thing I'm unsure about is the keybindings. All the F keys are popular, even with modifiers. You might end up masking some other package, or the other way around. That's why we recommend to not ship with bindings enabled and instead tell users how to set their own.
  • Hopefully you also tested with some more recent builds of Sublime Text? The one in your readme is ancient.

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

braver commented Dec 16, 2023

👋🏻

@maliayas
Copy link
Contributor Author

Oh, I didn't notice the feedback.

  1. You are right, I'll update the readme.
  2. Keybindings which this plugin uses are already defined by the original Xdebug plugin. I only override them. So I think that's fair?
  3. I'll do once I find time and get back to you.

Thanks for the review.

@braver
Copy link
Collaborator

braver commented Dec 16, 2023

So I think that's fair?

Certainly 👍🏻

Let me know!

@braver braver closed this Feb 15, 2024
@braver braver added timeout A pull request needed changes but was not updated in time (2 weeks after becoming stale) and removed stale The pull request needs to be updated but has not been within the recent past (2 weeks) labels Feb 15, 2024
@braver
Copy link
Collaborator

braver commented Feb 15, 2024

get back to you.

let me know, we can re-open then

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.

3 participants