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

Refactor API access mode rotation #5822

Merged
merged 1 commit into from
Feb 19, 2024
Merged

Refactor API access mode rotation #5822

merged 1 commit into from
Feb 19, 2024

Conversation

dlon
Copy link
Member

@dlon dlon commented Feb 15, 2024

Fix DES-635.


This change is Reviewable

Copy link

linear bot commented Feb 16, 2024

@dlon dlon changed the title Refactor communication between the REST client and API access mode actor Refactor API access mode rotation Feb 16, 2024
@dlon dlon marked this pull request as ready for review February 16, 2024 13:41
Copy link
Contributor

@MarkusPettersson98 MarkusPettersson98 left a comment

Choose a reason for hiding this comment

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

Very nice! Left som very minor comments 😊

Reviewed 3 of 10 files at r1, 8 of 8 files at r2, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @dlon)


mullvad-api/src/lib.rs line 181 at r2 (raw file):

            force_direct: force_direct
                .map(|force_direct_env| force_direct_env.to_lowercase() != "0")
                .unwrap_or(false),

Is this supposed to be changed in this PR? 😊

Code quote:

            force_direct: force_direct
                .map(|force_direct_env| force_direct_env.to_lowercase() != "0")
                .unwrap_or(false),

mullvad-daemon/src/api.rs line 275 at r2 (raw file):

                        let mut new_setting = setting.clone();
                        new_setting.enable();
                        new_setting

Compile error - simply remove the return value

Code quote:

                        new_setting

mullvad-daemon/src/api.rs line 489 at r2 (raw file):

            }
            None => {
                // Current method was removed: A new access mehtod will suddenly have the same index as the one

Typo in old comment 😄

Code quote:

mehtod

@dlon dlon force-pushed the refactor-access-mode-tx branch from bd6fe5c to e471d07 Compare February 16, 2024 15:38
Copy link
Member Author

@dlon dlon left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 2 files at r3, all commit messages.
Reviewable status: 9 of 11 files reviewed, 2 unresolved discussions (waiting on @dlon and @MarkusPettersson98)


mullvad-api/src/lib.rs line 181 at r2 (raw file):

Previously, MarkusPettersson98 (Markus Pettersson) wrote…

Is this supposed to be changed in this PR? 😊

Nope! Fixed


mullvad-daemon/src/api.rs line 275 at r2 (raw file):

Previously, MarkusPettersson98 (Markus Pettersson) wrote…

Compile error - simply remove the return value

Done.

Copy link
Contributor

@MarkusPettersson98 MarkusPettersson98 left a comment

Choose a reason for hiding this comment

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

Reviewed all commit messages.
Reviewable status: 9 of 11 files reviewed, all discussions resolved (waiting on @dlon)

@dlon dlon merged commit 10b4df8 into main Feb 19, 2024
43 of 47 checks passed
@dlon dlon deleted the refactor-access-mode-tx branch February 19, 2024 08:28
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