sbcgua / abap-package-version-shield

shield.io lambda to detect version of abap package, serialized by abapGit
https://sbcgua.github.io/abap-package-version-shield
8 stars 4 forks source link

Error when using files in ABAP namespace #117

Closed mbtools closed 2 years ago

mbtools commented 2 years ago

I would like to use this file as a reference: https://github.com/Marc-Bernard-Tools/MBT-Base/blob/main/src/tools/%23mbtools%23cl_tool_bc.clas.abap#L19

When I try with this: https://shield.abap.space/version-shield-json/github/Marc-Bernard-Tools/MBT-Base/blob/main/src/tools/%23mbtools%23cl_tool_bc.clas.abap

I get error "[file] has disallowed symbols"

Looks like % is missing here:

https://github.com/sbcgua/abap-package-version-shield/blob/6287d262dc53196e706c834d1d608fc4f6d1cde2/lib/params.js#L75

sbcgua commented 2 years ago

namespaces, yeah. It's not just %, it should be %DD (digit digit). Probably I need to unescape the params before validation. Will have a look during the weekend. Also PR is welcomed, if you have time earlier :)

sbcgua commented 2 years ago

Finally got my hands on it. Sorry for so long silence. The issue is kind of fixed (dev version) but there's another issue - non-master branches. Appears that master is hardcoded and thus implicitly supposed. I mean that there is no way to pass another name at the moment so changing the logic will break all existing use cases. Need to think about it a bit. One option is to auto check main on 1st failure. Another is to add another optional param to url command. Not sure which is easier...

e.g. github/Marc-Bernard-Tools/MBT-Base/!main/src/tools/%23mbtools%23cl_tool_bc.clas.abap

sbcgua commented 2 years ago

Maybe fallback to main makes more sense and actually easier ...

sbcgua commented 2 years ago

Ah no, master is automatically converted to main by github ... seems like this kind of constant definition does not fit

    CONSTANTS:
      BEGIN OF c_tool,
        version      TYPE string VALUE '1.2.0' ##NO_TEXT,

because of begin of ...

Hmm that could be difficult. Would you consider changin it to constants version ... style ?

Then try https://dev.shield.abap.space/version-shield-json/github/Marc-Bernard-Tools/MBT-Base/src/tools/%23mbtools%23cl_tool_bc.clas.abap (note dev.)

sbcgua commented 2 years ago

reworked the parser. try now. On dev instance @mbtools

https://dev.shield.abap.space/version-shield-json/github/Marc-Bernard-Tools/MBT-Base/src/tools/%23mbtools%23cl_tool_bc.clas.abap

If OK, I'll deploy to prod

mbtools commented 2 years ago

thanks for working on this :) my tool framework is based on a structure, so not something I could have changed easily.

shields.io seems to be down currently. I will check when it's back up.

mbtools commented 2 years ago

your part is working great. thx.

there is however an issue with shields.io when the url parameter contains escaped characters

https://img.shields.io/endpoint?url=https://dev.shield.abap.space/version-shield-json/github/Marc-Bernard-Tools/MBT-Base/src/tools/%23mbtools%23cl_tool_bc.clas.abap

image

I will create an issue with them.

sbcgua commented 2 years ago

Hmm, that's unexpected ... :) Ok, I'll deploy the code to production in the meantime because it has some other small improvements. If there is anything to fix more we can do it afterwards. Let's keep this issue open

mbtools commented 2 years ago

https://github.com/badges/shields/issues/7757

mbtools commented 2 years ago

One has to escape the complete URL, of course. Works well with dev now: https://github.com/Marc-Bernard-Tools/MBT-Base/blob/main/README.md

sbcgua commented 2 years ago

Escape escaped URL ... nice :)) So ... can be closed ?