Addon-manager: pyrate: changed repo url

[joha2]

Hey @luzpaz and @yorikvanhavre

I changed the repo url of pyrate, since we moved to salsa.debian.org. I hope that editing the .gitmodules file directly is the correct way to do this. If this is not the case, please feel free to reject the PR and I will perform the changes in an appropriate manner. (But you have to tell me, how to do it :-))

Best wishes
Johannes

[luzpaz]

Yorik will the Addon Manager work with salsa.debian as a got url?

[chennes]

No - I can add it in 0.20, then we can talk about backporting to 0.19.

[joha2]

Hi @luzpaz and @chennes

Thanks for the fast response! From a short look at the addon manager code in the FreeCAD main repo, I saw that some of the functions assume that the hosting platform is either gitlab, github or framagit. These functions concerning the artifacts extracted from the repos (like README.md and the zip file) could be problematic, if another platform is used. Is this correct? Should I provide the appropriate paths?

Best wishes
Johannes

[chennes]

Yes, that's correct -- we're trying to use the web interfaces to those sites to access some specific raw files and/or blobs, and there are really two different "standard" ways of doing those things. Salsa appears to use a mix of the GitHub and Gitlab styles. And I couldn't find an appropriate way top extract the HTML Readme.md data from Salsa, so the best option there is going to be to use the Markdown on FreeCAD 0.19, and add a package.xml for 0.20.

[joha2]

Ah that is not good. So could you please elaborate, what is meant by "use the Markdown on 0.19 and add a package.xml for 0.20"? I did not follow FreeCAD development recently due to my quite old Mint 18.3 Linux and the fact that the freecad-daily is not working for it anymore. (Compiling myself is also difficult due to old dependencies)

[chennes]

Well, it's not terrible, it works fine if you have the markdown python library installed. But if you don't you're just presented with the raw markdown. Not unusable, but not a great user experience. In the 0.20 development cycle we are trying to move away from displaying html we've scraped off the git host, and to a using a metadata file that contains additional information about the repository. You can find information about that file here: https://wiki.freecadweb.org/Package_Metadata

[joha2]

Hey @chennes thanks for the hints! So at the end it is up to the user, whether the markdown library is installed on their system. Maybe from an UI perspective one can only provide a warning once the import is failed. For the package.xml file I implemented a first suggestion how it could look like in our salsa repo. Could you please have a look, whether this is OK?

https://salsa.debian.org/joha2/pyrate/-/blob/master/package.xml

A few comments:

• I added a <comment> tag to provide the information, that the repo should be cloned recursively. Obviously this is not the right place for this information. Do you see a possibility to perform the cloning recursively? (Either by a specific tag in the XML file or by using git clone --recursive ... as a default?)
• I hope the classname is correct. I used the class which is derived from FreeCADGui.Workbench.
• I left the workbench tag nearly empty, since all the necessary things like icon and so on are defined at top level already (maintainer = author in our case).
• The path to the icon is given in a relative form, do I need to use ./ in front?

I also saw, that you already provided code in one of the PRs for the addon manager in the main repo of FreeCAD due to my PR here. Thank you very much for that!

Best wishes
Johannes

[chennes]

A few points:

1. Recursive cloning is the default, so the Addon Manager will take care of it, you don't need to do anything for it to work.
2. I think that classname is correct.
3. You are missing the "" tag from your workbench: it is probably "Pyrate" or "Pyrate Workbench" (however you want it to appear in the menu).
4. If you are only going to specify the icon in one place, put it as part of the workbench, rather than the package (the package will search for its icon in the workbenches if there isn't one specified, but workbenches require an icon).
5. Other than the icon, yes, it's correct that a content item will be mostly empty and inherit its information from the containing package.
6. Your icon path is correctly-formed.

[joha2]

Hey @chennes thanks for the review!

I moved the icon stuff between the workbench tags to reflect the necessity of the icon within workbench. What I did not understand is the "" tag, you mentioned. Did you refer to the name tag within the workbench, which I forgot? I added a name tag to workbench to be in line with your example given in the FreeCAD documentation. Another Question, which comes to my mind: Is it strictly necessary to provide a <freecadmax> tag? We do not intend to limit the usage of pyrate to some specific FreeCAD versions. Although, I did not test 0.20, yet and therefore cannot say, whether Pyrate is compatible with it.

Could you please have another look at my file?

Thank you for your help!

When the package.xml is OK, is it possible to accept the PR or are there other blocking points?

Best wishes
Johannes

PS: I think for the adoption of the package.xml it would be useful to provide some kind of linter, which checks whether the package.xml fulfills the standards. What do you think?

[luzpaz]

Just ping me when PR needs merging 😉

[chennes]

Yes, it was the name tag I was trying to refer to, but was thwarted by GitHub. The freecadmin and freecadmax tags are both optional, and are intended as informational for potential installers. The purpose of a freecadmax tag is that you could potentially distribute a package with two versions your workbench in it, one for older versions and one for newer, and the addon manager could handle which needed to go in.

And yes, a linter, as well as some other creation tools, are things I have in mind for the future.

[chennes]

I should also note that when this gets merged, no version of FreeCAD without the latest changes to Addon Manager (which haven't even been merged yet!) will be able to access pyrate. So I wouldn't be in a huge hurry for this! I'm considering back porting this support to 0.19.3, but haven't done so yet.

[joha2]

Hey @chennes and @luzpaz thanks for your help. I just removed the freecadmax tag, since it makes no sense as far as I know. Furthermore, for me the goal is that pyrate works frictionless in FreeCAD without any difficulties (although the GUI is not there and the API is not stabilized, yet). Therefore, the earlier we have the package.xml, the better. 😄 The most urgent task, anyway, is to update the repo link. So if everything is OK from your side @luzpaz you may merge this PR. I hope that, although the backporting of the package.xml stuff seems not that easy, backporting the salsa-stuff easier. 😃

Again: Thank you for your fast help!

Best wishes
Johannes

[luzpaz]

Should we revisit this now?

[luzpaz]

@joha2 addon manager can support salsa: see #80 (comment)

[chennes]

Yes, since 0.19.3 now also supports salsa, I think that as long as @joha understands that previous FreeCAD versions won't be able to install this addon after we merge this, then it's OK to move forward.

[joha2]

Sorry for my long response time! At the end, this should no big problem, since one can use multiple FreeCAD versions side by side. So thanks for making the necessary changes. I will close this issue.
Oops. This was a pull request. I reopened it. Sorry for the confusion.