Add GitHub importer - #1658
Add GitHub importer#1658
Conversation
|
Also, for now I used FamixJavaFoldersImporter. But I think that @Gabriel-Darbord is working on import also with Nexus? Maybe in the future we could unify |
Gabriel-Darbord
left a comment
There was a problem hiding this comment.
Thanks @jecisc, this doesn't conflict with MooseNexus, they would even go well together.
For this review, I didn't go into technical details or tried it myself, it looks sound and you've probably tried it already, issues will arise if need be :)
I just have two remarks regarding where data is stored locally, and the choice of archive format.
Also, it's out of scope of this PR, but instead or in addition to having to use a token and potentially overwrite user credentials, GitHub has its own gh cli which could be used to make queries.
We would need a better infrastructure around it though, rather than just using LibC, so that's future work.
| archiveReference := self cacheDirectory / (aVersion folderName , '.tar.gz'). | ||
| archiveReference parent ensureCreateDirectory. | ||
| (ZnClient new | ||
| url: (repository downloadUrlForVersion: aVersion); | ||
| downloadTo: archiveReference pathString) ifFalse: [ self error: 'Cannot download the sources of ' , aVersion displayName ]. | ||
| LibC runCommand: 'tar -xzvf "' , archiveReference pathString , '" -C "' , aFolder pathString , '" --strip-components=1'. |
There was a problem hiding this comment.
Just noting that we could also use the .zip archive, which is more universally supported (I'm doubtful about Windows support for tarballs).
There's even a Pharo-native ZipArchive class that can be used instead of LibC.
At least, the return code of the command should be checked to give a clear error in case of failure.
There was a problem hiding this comment.
I don't have a strong opinion on this point.
Maybe the strongest way could be to use a zip, use libC to unzip since it's way faster than Pharo, and fallback to ZipArchive if there has been an error? (like an OS without unzip installed)
There was a problem hiding this comment.
I checked and tar seems to be shipped in windows since Win10 1803 (bsdtar). This could be enough?
There was a problem hiding this comment.
Then I guess tar is fine.
I would still verify the result of the command to give a clear error in case anything happens, rather than letting it fail silently, then getting potentially hard-to-debug errors down the line.
|
I notice you used |
|
Ah OK, I was misled by a test that made me think this was part of the structure. |

This PR propose to add a github importer for Moose.
This is a first version with limitations.
In the future I'd like to add pragmas to each Famix importer to make them discoverable and we could delegate the request of additional info to the importers depending on the language.
Also we can provide a github token to lift the API limit rate
But it will be for another time.