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

Update option A documentation with needed changes to releases section #139

Merged
merged 3 commits into from
Aug 12, 2022
Merged

Update option A documentation with needed changes to releases section #139

merged 3 commits into from
Aug 12, 2022

Conversation

zaid
Copy link
Contributor

@zaid zaid commented Jul 28, 2022

Update the Application Start Behaviour section in the README with the needed changes to the releases configuration when choosing Option A.

Resolves #133

Copy link
Owner

@tompave tompave left a comment

Choose a reason for hiding this comment

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

Thank you for this!

A couple of questions:

  1. Adding the releases: [ ... ] key to the project(): is it required for both runtime: false and app: false options? It's not clear.
  2. Is this not required at all with Option B?

@zaid
Copy link
Contributor Author

zaid commented Aug 2, 2022

Thank you for this!

@tompave my pleasure, thank you for your great work on this library!

A couple of questions:

  1. Adding the releases: [ ... ] key to the project(): is it required for both runtime: false and app: false options? It's not clear.

Great question, I just checked and it seems that this is needed for app: false as well so I'll update the PR with the needed changes.

  1. Is this not required at all with Option B?

I just checked and it looks like Option B doesn't require these changes.

@tompave
Copy link
Owner

tompave commented Aug 2, 2022

Awesome, thank you.

When you amend the PR, can you please specify that this is only required when using releases? i.e. not necessary if running with Mix?

@zaid
Copy link
Contributor Author

zaid commented Aug 2, 2022

When you amend the PR, can you please specify that this is only required when using releases? i.e. not necessary if running with Mix?

@tompave sure thing, I just updated the PR with the needed changes. Please let me know if I need to tweak this further.

@nathanalderson
Copy link

It would be helpful to note that this is also needed for fun_with_flags_ui if that is being used, especially since that README references this one.

@tompave
Copy link
Owner

tompave commented Aug 12, 2022

Thank you for this work!
@nathanalderson I think that's fine. At the end of the section the Readme mentions that FWF.UI also needs to be configured differently.

@tompave tompave merged commit 5622d66 into tompave:master Aug 12, 2022
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.

The module FunWithFlags.Supervisor was given as a child to a supervisor but it does not exist
3 participants