launchpad-reviewers team mailing list archive

[Simone Pelosi <mp+480177@xxxxxxxxxxxxxxxxxx>]

Great work! It looks good! I have just one question: should we use jinja templates for the generation of the settings.xml file ? I would make the generation cleaner, easier to read, and more maintainable. Something like: 

```
<?xml version="1.0" encoding="UTF-8"?>
<settings xmlns="http://maven.apache.org/SETTINGS/1.0.0";
          xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
          xsi:schemaLocation="http://maven.apache.org/SETTINGS/1.0.0 http://maven.apache.org/xsd/settings-1.0.0.xsd";>
    <servers>
        {% for name, repo in repositories.items() if "username" in repo and "password" in repo %}
        <server>
            <id>{{ name }}</id>
            <username>{{ repo.username }}</username>
            <password>{{ repo.password }}</password>
        </server>
        <server>
            <id>{{ name }}-snapshots</id>
            <username>{{ repo.username }}</username>
            <password>{{ repo.password }}</password>
        </server>
        {% endfor %}
    </servers>
    <profiles>
        {% for name, repo in repositories.items() if "url" in repo %}
        <profile>
            <id>{{ name }}</id>
            <repositories>
                <repository>
                    <id>{{ name }}</id>
                    <name>{{ name }}</name>
                    <url>{{ repo.url }}</url>
                    <snapshots>
                        <enabled>false</enabled>
                    </snapshots>
                </repository>
                <repository>
                    <id>{{ name }}-snapshots</id>
                    <name>{{ name }}</name>
                    <url>{{ repo.url }}</url>
                    <snapshots>
                        <enabled>true</enabled>
                    </snapshots>
                </repository>
            </repositories>
            <pluginRepositories>
                <pluginRepository>
                    <id>{{ name }}</id>
                    <name>{{ name }}</name>
                    <url>{{ repo.url }}</url>
                    <snapshots>
                        <enabled>false</enabled>
                    </snapshots>
                </pluginRepository>
                <pluginRepository>
                    <id>{{ name }}-snapshots</id>
                    <name>{{ name }}</name>
                    <url>{{ repo.url }}</url>
                    <snapshots>
                        <enabled>true</enabled>
                    </snapshots>
                </pluginRepository>
            </pluginRepositories>
        </profile>
        {% endfor %}
    </profiles>
    <mirrors>
        {% for name, repo in repositories.items() if "url" in repo %}
        <mirror>
            <id>{{ name }}</id>
            <name>Maven Repository Manager running on {{ name }}</name>
            <url>{{ repo.url }}</url>
            <mirrorOf>*</mirrorOf>
        </mirror>
        {% endfor %}
    </mirrors>
</settings>
```
-- 
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.