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

Prefixing all code names of commands with 'nbls'. #6218

Conversation

lahodaj
Copy link
Contributor

@lahodaj lahodaj commented Jul 18, 2023

Another attempt to get:
#6015

Adding support for changing the prefix of code names of commands and configuration options from on the client extension.


^Add meaningful description above

By opening a pull request you confirm that, unless explicitly stated otherwise, the changes -

  • are all your own work, and you have the right to contribute them.
  • are contributed solely under the terms and conditions of the Apache License 2.0 (see section 5 of the license for more information).

Please make sure (eg. git log) that all commits have a valid name and email address for you in the Author field.

If you're a first time contributor, see the Contributing guidelines for more information.

If you're a committer, please label the PR before pressing "Create pull request" so that the right test jobs can run.

@neilcsmith-net
Copy link
Member

@lahodaj please use the GitHub account tied to your ASF membership. Commit email should ideally be tied to that too.

@neilcsmith-net neilcsmith-net added do not merge Don't merge this PR, it is not ready or just demonstration purposes. and removed do not merge Don't merge this PR, it is not ready or just demonstration purposes. labels Jul 18, 2023
@lahodaj lahodaj changed the base branch from master to vsnetbeans_1903 September 15, 2023 08:27
@lahodaj lahodaj added the do not merge Don't merge this PR, it is not ready or just demonstration purposes. label Sep 15, 2023
@lahodaj lahodaj changed the base branch from vsnetbeans_1903 to master October 10, 2023 12:21
Copy link
Contributor

@dbalek dbalek left a comment

Choose a reason for hiding this comment

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

Looks fine to me.

@lahodaj lahodaj force-pushed the lsp-server-generic-commands-settings-try2 branch from cbb6f97 to f6e7f94 Compare October 16, 2023 15:42
@lahodaj lahodaj removed do not merge Don't merge this PR, it is not ready or just demonstration purposes. Need Squashing labels Oct 17, 2023
@lahodaj lahodaj force-pushed the lsp-server-generic-commands-settings-try2 branch from f6e7f94 to 96e50f5 Compare October 17, 2023 06:52
@arvindaprameya
Copy link

Approved, thank you for making this change

@lahodaj lahodaj changed the base branch from master to delivery October 17, 2023 09:10
@lahodaj
Copy link
Contributor Author

lahodaj commented Oct 17, 2023

@neilcsmith-net, it would be awesome if this could be included in NB 20. I am sorry I am late, but there's unfortunately only a small window in which we can get this patch in (as it will break some outside components, so it is necessary give them time to adjust), and I unfortunately was not able to finish this in the past 2-3 weeks. Thanks for your consideration!

@neilcsmith-net
Copy link
Member

neilcsmith-net commented Oct 17, 2023

@lahodaj not my call to make. cc./ @MartinBalin

Everything not in by branch just gets automatically bumped to the next milestone. Whether it belongs in delivery is then up to review. As there is almost nothing here touching code outside of LSP then he's best placed on the release team to oversee that. See also https://cwiki.apache.org/confluence/display/NETBEANS/Pull+requests+for+delivery

There appears to be git merge conflict text included here!

@neilcsmith-net neilcsmith-net added the LSP [ci] enable Language Server Protocol tests label Oct 17, 2023
@apache apache locked and limited conversation to collaborators Oct 17, 2023
@apache apache unlocked this conversation Oct 17, 2023
@neilcsmith-net neilcsmith-net added this to the NB20 milestone Oct 17, 2023
@neilcsmith-net neilcsmith-net added the do not merge Don't merge this PR, it is not ready or just demonstration purposes. label Oct 17, 2023
@lahodaj lahodaj force-pushed the lsp-server-generic-commands-settings-try2 branch from 96e50f5 to 5ef2d77 Compare October 17, 2023 10:02
@lahodaj lahodaj force-pushed the lsp-server-generic-commands-settings-try2 branch from 5ef2d77 to b6dc0d4 Compare October 17, 2023 10:22
@neilcsmith-net neilcsmith-net added the VSCode Extension [ci] enable VSCode Extension tests label Oct 17, 2023
@apache apache locked and limited conversation to collaborators Oct 17, 2023
@apache apache unlocked this conversation Oct 17, 2023
@neilcsmith-net neilcsmith-net removed the do not merge Don't merge this PR, it is not ready or just demonstration purposes. label Oct 17, 2023
@lahodaj
Copy link
Contributor Author

lahodaj commented Oct 18, 2023

@neilcsmith-net OK to merge, or do you do merging to delivery? (Sorry, the process is not completely clear to me.) Thanks!

@neilcsmith-net
Copy link
Member

neilcsmith-net commented Oct 18, 2023

@lahodaj someone on the release team will merge. We handle all merging to delivery. It's just not our call (any more than any other reviewer) what should be included.

Key line from the wiki page above - "The release team handles all merging to delivery. The release team does not control what gets merged. All pull requests for delivery that pass review will be merged for the next release candidate if there is one."

This will get merged for 20-rc2, but not immediately. First priority is to get a fix in for that fact that the VSCode plugin isn't buildable from the release bundle - see comment on #6417

@neilcsmith-net neilcsmith-net merged commit e0e448f into apache:delivery Oct 23, 2023
36 of 62 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LSP [ci] enable Language Server Protocol tests VSCode Extension [ci] enable VSCode Extension tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants