Skip to content

https://github.com/bonfire-networks/bonfire-app/issues/1651#1652

Merged
mayel merged 1 commit into
bonfire-networks:mainfrom
ju1m:fix-1651
Dec 24, 2025
Merged

https://github.com/bonfire-networks/bonfire-app/issues/1651#1652
mayel merged 1 commit into
bonfire-networks:mainfrom
ju1m:fix-1651

Conversation

@ju1m
Copy link
Copy Markdown
Contributor

@ju1m ju1m commented Nov 19, 2025

This PR makes it so that mix.exs installs flavour if it is different than base_flavour,
base_flavour being the case handled just before that.
This instead of installing flavour if it is different than default_flavour which makes little sense to me here.

Possibly fix partially: #1651

@mayel
Copy link
Copy Markdown
Member

mayel commented Nov 27, 2025

I'm not following the logic here, since the code right above the change handles the possible inclusion of the base_flavour while this code handles the possible inclusion of the current flavour?

@ju1m
Copy link
Copy Markdown
Contributor Author

ju1m commented Nov 27, 2025

Well, let flavour = "social", then flavour != default_flavour is always false, and thus {flavour_atom, git: "https://github.com/bonfire-networks/#{flavour}", override: true} is never added to the deps.
In other words, there should be two blocks:

  1. one to install base_flavour (which is already there),
  2. and one to install flavour, when it's different than base_flavour because the base_flavour case is already handled by 1..
    The test whether to include 2. or not must therefore be flavour != base_flavour, not flavour != default_flavour which makes no sense because default_flavour is not what 1. installs.

@mayel
Copy link
Copy Markdown
Member

mayel commented Dec 24, 2025

I got it now, thanks again!

@mayel mayel merged commit af471d6 into bonfire-networks:main Dec 24, 2025
@ju1m ju1m deleted the fix-1651 branch December 24, 2025 15:00
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.

2 participants