Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support elixir #82

Merged
merged 8 commits into from
Sep 22, 2022
Merged

Support elixir #82

merged 8 commits into from
Sep 22, 2022

Conversation

mattwynne
Copy link
Member

@mattwynne mattwynne commented Sep 20, 2022

🤔 What's changed?

  • Added support for bumping the version number in elixir projects

⚡️ What's your motivation?

#80 => cucumber/messages#46 => cucumber/messages#45 => cucumber/common#2029

🏷️ What kind of change is this?

  • ⚡ New feature (non-breaking change which adds new behaviour)

♻️ Anything particular you want feedback on?

The fixture was generated using mix new which made a project with a version string value. When I look at the code from cucumber/messages I see we've used a @vsn attribute(?) on the module.

I've decided to go with what mix new generated, and I think that, unless there's a good reason, we should change cucumber/messages to hardcode the version in the project as in the generated file.

@WannesFransen1994 do you see any problem with this? Any idea why the @vsn attribute was used originally?

📋 Checklist:

  • I agree to respect and uphold the Cucumber Community Code of Conduct
  • I've changed the behaviour of the code
    • I have added/updated tests to cover my changes.
  • My change requires a change to the documentation.
    • I have updated the documentation accordingly.
  • Users should know about my change
    • I have added an entry to the "Unreleased" section of the CHANGELOG, linking to this pull request.

This text was originally generated from a template, then edited by hand. You can modify the template here.

@mattwynne
Copy link
Member Author

In cucumber/messages#46 I took the decision to change the mix.exs file to just have the version inline in the project instead of in a @vsn field. So I think this version of polyglot-release should work with it.

But we won't know until we try it.

polyglot-release Outdated Show resolved Hide resolved
Copy link
Contributor

@mpkorstanje mpkorstanje left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm seeing quite a few fixtures for Elixer, yet the release proces only involves a single file. I don't immediately see what those fixtures are for. Perhaps they can be cleaned up.

@mattwynne
Copy link
Member Author

I'm seeing quite a few fixtures for Elixer, yet the release proces only involves a single file. I don't immediately see what those fixtures are for. Perhaps they can be cleaned up.

Yeah, I just used their mix new command to create the fixture. I don't have enough knowledge of the language to know what could be cleaned up. Maybe @WannesFransen1994 could help.

@WannesFransen1994
Copy link

Regarding the @vsn, this made it a bit easier to bump the version with the makefile. It can be perfectly hardcoded and adjusted with makefiles/scripts.

As for the fixtures, I'm not completely up to date what "technical flow" would be. Is this a file that'd be copied to the respective messages/elixir folder after which the release process would be started there? Or is there a reason why a new project would be generated with mix new?

@mattwynne
Copy link
Member Author

I'm seeing quite a few fixtures for Elixer, yet the release proces only involves a single file. I don't immediately see what those fixtures are for. Perhaps they can be cleaned up.

Yeah, I just used their mix new command to create the fixture. I don't have enough knowledge of the language to know what could be cleaned up. Maybe @WannesFransen1994 could help.

Thinking about it, given we only touch the mix.exs file, we only need that one, even if it's not a realistic/build-able elixir project.

@mpkorstanje mpkorstanje enabled auto-merge (squash) September 22, 2022 15:04
@mpkorstanje mpkorstanje merged commit 60b1b10 into main Sep 22, 2022
@mpkorstanje mpkorstanje deleted the support-elixir branch September 22, 2022 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support hex (Elixir / Erlang)
3 participants