mamba-org / devcontainer-features

MIT License
4 stars 2 forks source link

Rewrite the micromamba Feature #3

Closed maresb closed 1 year ago

maresb commented 1 year ago

Hi @eitsupi, I'm really sorry it took me so long to review this.

Could you please take a look at my comments when you get the chance? (I made them inline in the script.)

Thanks so much for your patience.

maresb commented 1 year ago

@eitsupi, what do you think of the changes I'm proposing here? Do they conform with the standard? Are they a good idea? Thanks!

maresb commented 1 year ago

Also, could you do package installs using micromamba in the tests?

Yes, I'll start working on this.

And, could you increment the version number to 0.1.0?

I could, but I'm confused about the release process. How does one know which commit corresponds to a release? I feel like we should create a tag in the repository. Would it make sense to postpone the version increment until we are ready for release? Then on the final commit, we increment the number, tag it, and execute the build workflow? Or is there a better way?

maresb commented 1 year ago

I added tests for the addCondaForge option and a test for installing the python package via micromamba. I look forward to your responses regarding the remaining open points.

Regarding the soft version matching, I opened https://github.com/mamba-org/mamba/issues/2164. I think it would be easy to implement it there, if they make it open.

eitsupi commented 1 year ago

I could, but I'm confused about the release process. How does one know which commit corresponds to a release? I feel like we should create a tag in the repository. Would it make sense to postpone the version increment until we are ready for release? Then on the final commit, we increment the number, tag it, and execute the build workflow? Or is there a better way?

Good point. Unfortunately, at the moment there is no way to do it! See devcontainers/features#113.

As far as I know, there are currently no Features that be managed releases by tags, and once devcontainers/action#112 is merged and GitHub Actions does this, management by tags will become commonplace.......

If this is wanted now, the maintainer can manually push the tags after the release, since the release workflow is executed manually. https://github.com/eitsupi/mamba-devcontainer-features/blob/12349ea7a31cba62e9a7b28392e51a0b4acc688d/.github/workflows/release.yaml#L2-L3

maresb commented 1 year ago

once https://github.com/devcontainers/action/pull/112 is merged and GitHub Actions does this, management by tags will become commonplace

That looks great! Can we anticipate the format of the tags? From this line

    const tag = `${type}_${id}_v${version}`;

it's not clear to me what are type and id.

eitsupi commented 1 year ago

That looks great! Can we anticipate the format of the tags? From this line

    const tag = `${type}_${id}_v${version}`;

it's not clear to me what are type and id.

Based on the following line, it would be something like feature_micromamba_v0.1.0. https://github.com/devcontainers/action/blob/09709bc562da6615f791c204928d1415294c647e/src/main.ts#L66

However, this is not finalized, so it is better to use this for now and push the tag again with the new format if the format changes.

maresb commented 1 year ago

For testing purposes, I made a release in my fork:

    "features": {
        "ghcr.io/maresb/mamba-devcontainer-features/micromamba:latest": {
            "channels": "conda-forge,defaults"
        }
    }

When you're satisfied with all the changes, could we merge this PR and then do the release in a separate commit?

maresb commented 1 year ago

I think your excellent insight and lots of work made this Feature great!

Yes, thank you so much for your patience and for teaching me so many things! I have learned a lot from you in this PR.

eitsupi commented 1 year ago

Thanks for the PR this has been really great, so let me know if you want to transfer to this repo to mamba-org.

maresb commented 1 year ago

transfer to this repo to mamba-org

I think this is a good idea, so I will propose this on Gitter in mamba-org. But in order that people can evaluate it, could you make a package under this repo?

eitsupi commented 1 year ago

could you make a package under this repo?

Of course. However, to experience the release process, why don't you push the workflow trigger after #5 is merged?

image

maresb commented 1 year ago

Got it! Shall I merge and release right now?

eitsupi commented 1 year ago

Yes, please try it!

maresb commented 1 year ago

I have proposed this in Gitter: https://gitter.im/mamba-org/Lobby?at=6399afca8bdea01368b61508