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

Modify fairings #2598

Merged
merged 1 commit into from
Aug 21, 2024
Merged

Modify fairings #2598

merged 1 commit into from
Aug 21, 2024

Conversation

the10thWiz
Copy link
Collaborator

@the10thWiz the10thWiz commented Aug 14, 2023

Supersedes: #2271
Resolves: #2271

Adds a modify_fairings method to make changes to fairings before launch.

There were several comments on the original PR, which have been resolved. Some simplification of the API has been performed.

@SergioBenitez SergioBenitez force-pushed the modify-fairings branch 2 times, most recently from 7f969ec to 1cafc72 Compare August 21, 2024 09:16
@SergioBenitez
Copy link
Member

I went a bit of a different direction with this one that I think results in a nicer API that can be used in more situations. It necessitated a bit of tricky code in the fairings set, but thankfully that complexity doesn't leak to the user, and there's no unsafe anywhere. Let me know what you think.

Introduces four new methods:

  * `Rocket::fairing::<F>()`
  * `Rocket::fairing_mut::<F>()`
  * `Rocket::fairings::<F>()`
  * `Rocket::fairings_mut::<F>()`

These methods allow retrieving references to fairings of type `F` from
an instance of `Rocket`. The `fairing` and `fairing_mut` methods return
a (mutable) reference to the first attached fairing of type `F`, while
the `fairings` and `fairings_mut` methods return an iterator over
(mutable) references to all attached fairings of type `F`.

Co-authored-by: Matthew Pomes <[email protected]>
Copy link
Collaborator Author

@the10thWiz the10thWiz left a comment

Choose a reason for hiding this comment

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

I like the changes. My original PR was intended to work with a specific use-case, but I think the final API works just fine.

The only thing I see is that you can technically call fairings_mut on a Rocket<Orbit>, but I don't think there's any point during the Rocket lifecycle this is actually possible outside of Rocket. I don't think this is an issue, since there isn't much you could do at that point anyway.

(Btw, I'd post an approval, but I can't approve my own PR).

@SergioBenitez SergioBenitez merged commit d332339 into rwf2:master Aug 21, 2024
16 checks passed
@the10thWiz the10thWiz deleted the modify-fairings branch August 24, 2024 01:06
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