← Back to team overview

launchpad-reviewers team mailing list archive

Re: [Merge] ~ruinedyourlife/launchpad-buildd:process-craft-env-vars into launchpad-buildd:master

 

> I lack the background knowledge about maven and cargo to be able to review
> this MP.
> 
> In general, it would be great to add links to the relevant pieces of
> documentation, but in the the code as a comment and in the MP, and also
> explain the important pieces in the commit message.
> 
> Please, also go through your changes and add helpful comments where you use
> special knowledge about maven or cargo, e.g. "create .m2 directory" - I do not
> know what that is.
> 
> Where is the convention of CARGO_ and MAVEN_ prefixes coming from? If this is
> a custom thing, we need to document that, if that is coming from the
> languages, it would make sense to add links to the relevant documentation
> pages.

https://docs.google.com/document/d/162mxdleiJALK6-CtMQgzmLqJ2UWC1bAVuKc03MrUSvI/edit?tab=t.0

this covers a bit of the topic with links to the relevant documentation (at the bottom)

cargo has a specific format which needs the env variables to look a certain way to be recognized
maven doesnt have such a requirement, it instead uses the creation of a `settings.xml` file in the `.m2` directory

so i used the same format we used for pip to pass the vars in the other MP, and i then process them accordingly to what is needed by cargo or maven in this MP (variables that look a certain way for cargo, and the settings file for maven)
-- 
https://code.launchpad.net/~ruinedyourlife/launchpad-buildd/+git/launchpad-buildd/+merge/480177
Your team Launchpad code reviewers is requested to review the proposed merge of ~ruinedyourlife/launchpad-buildd:process-craft-env-vars into launchpad-buildd:master.



References