reconquest / atlassian-external-hooks

External Hooks plugin for Atlassian Bitbucket
https://external-hooks.reconquest.io
Other
44 stars 37 forks source link

Change description key #70

Open DrVanScott opened 5 years ago

DrVanScott commented 5 years ago

Just a small issue:

https://github.com/reconquest/atlassian-external-hooks/blob/ff92abfcc27ceee688126511555710ded659f44c/src/main/resources/atlassian-plugin.xml#L39

Better change

i18n-name-key="external-async-post-receive-hook.name"

to

i18n-name-key="external-post-receive-hook.name"

Accordingly change the in place description from

The External Async Post Receive Hook Plugin

to (external-hooks.properties)

The External Post Receive Hook Plugin

or vice versa?

seletskiy commented 5 years ago

@DrVanScott: It's named Async just to be clear that hook is not guaranteed to run exactly after push and it can't be used to reject push at all due the way hooks are executed in Atlassian Bitbucket.

DrVanScott commented 5 years ago

@seletskiy: I understand and that's why i wrote "or vice verse". The main point of my argument is the key name. It does not match the key in external-hooks.properties nor does it match the naming scheme.

Look at line 27: https://github.com/reconquest/atlassian-external-hooks/blob/ff92abfcc27ceee688126511555710ded659f44c/src/main/resources/atlassian-plugin.xml#L27

Attribute "i18n-name-key" and "key" are aligned (Same for the description key in the next line) This is not the case for the async case.

A minor side effect of this is: The description for the first (and last) hook is taken from the properties file, while the description of async hook is taken from the xml, due to no match in the property file.

Not a big deal, just something against my pedantic mind... ;-)

seletskiy commented 5 years ago

@DrVanScott: ah, I see what you mean now. I'm not sure however that it can be changed safely, because Bitbucket may store settings for hooks by it's key and this change may not be backward compatible.

DrVanScott commented 5 years ago

I agree not to change "key", but it should be possible to change "i18n-name-key".

At least make a change in external-hook.properties to make this mechanism also work for the async hook.

Whatever solution you choose, you need to change the content of async's description in the property file.