The subject looks broken, both in for mail and git purpose. It got nested tags, and the rest is way to long - we try to stay under 70 characters per line for git commits, and the commit subject should be a very short summary, the rest has to move in the commit messages body.
Your subject: > [[PATCH pve-manager] Button Delete Subscription key 1/1] [pve-manager] Add > button to delete Subscription key. Delete Subcription key from > "/etc/subscription" Better one: > [PATCH pve-manager] api, ui: allow to remove subscription Here "api, ui: allow to remove subscription" is the real commit message subject and "[PATCH pve-manager]" is done by git send-email or format-patch --subject-prefix "PATCH pve-manager" See the "for patch formatting convenience" helper in: http://intranet.proxmox.com/index.php/Useful_Tips#copy.26paste_shortcut_to_fetch_most_repositories to set this as default in the configuration. On 4/20/20 2:00 PM, Moayad Almalat wrote: > Signed-off-by: Moayad Almalat <m.alma...@proxmox.com> > --- > PVE/API2/Subscription.pm | 22 ++++++++++++++++++++++ > PVE/CLI/pvesubscription.pm | 1 + > www/manager6/node/Subscription.js | 8 ++++++++ > 3 files changed, 31 insertions(+) > > diff --git a/PVE/API2/Subscription.pm b/PVE/API2/Subscription.pm > index 6657c00d..46895997 100644 > --- a/PVE/API2/Subscription.pm > +++ b/PVE/API2/Subscription.pm > @@ -245,4 +245,26 @@ __PACKAGE__->register_method ({ > return undef; > }}); > > +__PACKAGE__->register_method ({ > + name => 'delete', > + path => '', > + method => 'DELETE', > + permissions => { > + check => ['perm', '/nodes/{node}', [ 'Sys.Modify' ]], > + }, > + description => "Delete subscription key.", > + proxyto => 'node', > + protected => 1, > + parameters => { > + additionalProperties => 0, > + properties => { > + node => get_standard_option('pve-node'), > + > + }, > + }, > + code => sub { > + unlink ("/etc/subscription"); unlink can fail, which needs to be checked. Also, please no space between unlink and the parenthesis. unlink("/etc/subscription") or die "could not delete subscription key: $!"; Above uses the $! perl variable. This is for C library functions and would be the ernno (and it's derived string from that error number). Note that unlink does not fails if the file is not present before trying to unlink, that behavior is OK here. > + return undef; > + }}); > + > 1; > diff --git a/PVE/CLI/pvesubscription.pm b/PVE/CLI/pvesubscription.pm > index cd81c415..751dde58 100755 > --- a/PVE/CLI/pvesubscription.pm > +++ b/PVE/CLI/pvesubscription.pm > @@ -28,6 +28,7 @@ our $cmddef = { > } > }], > set => [ 'PVE::API2::Subscription', 'set', ['key'], { node => $nodename > } ], > + delete => [ 'PVE::API2::Subscription', 'delete', undef, { node => > $nodename } ], > }; > > 1; > diff --git a/www/manager6/node/Subscription.js > b/www/manager6/node/Subscription.js > index e4a35874..7f93cd80 100644 > --- a/www/manager6/node/Subscription.js > +++ b/www/manager6/node/Subscription.js > @@ -163,6 +163,14 @@ Ext.define('PVE.node.Subscription', { > win.on('destroy', reload); > } > }, > + { > + text: gettext('Delete Subscription Key'), This is a to long text, buttons should have rather shorter ones, e.g.: gettext('Remove Subscription') > + xtype: 'proxmoxStdRemoveButton', > + confirmMsg: 'Are you sure to delete Subscription Key?', misses the gettext, as is it cannot be translated.. Also grammar error, misses the article for subscription key, maybe use: gettext('Are you sure to remove the subscription key?') > + getUrl: () => '/api2/extjs' + baseurl, This misses the "selModel: false" and you can use baseurl now instead of overwriting getUrl. I do not want the behavior that I have to select a bogus row of the subscription view first for the delete button to get disabled, as said offlist ;-) > + dangerous: true, > + callback: reload,> + }, > { > text: gettext('Check'), > handler: function() { > _______________________________________________ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
