Conversation
gregors
left a comment
gregors
left a comment
There was a problem hiding this comment.
Very excited to get this in. But I have some questions regarding the removal of the publishing step. We also probably want to make sure elixir 1.13 is still covered. I also suggest using a v1.1.0 version for this since it's is in effect an addition and not a bug.
Happy to help move this along.
| - pair: | ||
| elixir: 1.15.6 | ||
| otp: 26.1 | ||
| lint: lint |
There was a problem hiding this comment.
we're missing elixir 1.13 in this build
There was a problem hiding this comment.
and might want to add a few more recent ones
| with: | ||
| files: | | ||
| ${{ steps.build-crate.outputs.file-path }} | ||
| if: startsWith(github.ref, 'refs/tags/') |
There was a problem hiding this comment.
We've mistakenly removed doc creation and hex publishing, unless I'm missing how this works. Probably need to bring that back unless the idea is that hex publishing becomes some sort of manual process.
| use Mix.Project | ||
|
|
||
| @source_url "https://github.com/Thomas-Jean/blake3" | ||
| @version "1.0.3" |
There was a problem hiding this comment.
I think there's enough changing in this that "1.1.0" is warranted
|
|
||
| - name: Check unused deps | ||
| run: mix deps.unlock --check-unused | ||
| if: ${{ matrix.lint }} |
There was a problem hiding this comment.
curious about the conditional lint, is the goal only to check the most recent pair?
There was a problem hiding this comment.
This just allows you to turn off linting for a specific pair if desired.
| target: ${{ matrix.job.target }} | ||
| nif-version: ${{ matrix.nif }} | ||
| use-cross: ${{ matrix.job.use-cross }} | ||
| cross-version: ${{ matrix.job.cross-version || 'v0.2.4' }} |
There was a problem hiding this comment.
why the mix between v0.2.4 and v0.2.5?
There was a problem hiding this comment.
I don't recall. I pretty much always end up copy/pasting my CI configs and probably need to revisit.
@gregors yes, makes sense. Essentially, this PR is pretty much exactly what I did in my fork in order to get something working for my own use-case. I haven't spent any time on automating hex publishing for any of my own packages, so I don't have any experience in department. There is also a step which is required to run before you publish the package so that the checksum file is included in the published package. This has to be run after the artifacts have been built and uploaded on GitHub. Currently, I run this manually before publishing so that I can commit it into the repo so it can be used outside of hex like I'm doing.
@gregors sounds good. I don't have a lot of time to run down all these changes at the moment. Feel free to close this, amend changes, or what ever you prefer. |
2 participants
As requested by @juulSme here