Skip to content

Conversation

notoriaga
Copy link
Contributor

The issue was during an import ntrip.enable would get written before the other settings, so if the config file had ntrip.enable=True you'd get an error trying to write username etc. This change writes that setting last (same for ethernet.interface_mode too, maybe there are more like this?). It also adds a little context to the error message during an import

@notoriaga notoriaga requested a review from a team January 5, 2022 21:31
}
}

fn sort_import_group<'a>(
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should use "reorganize" instead of "sort" and a description of why we must reorganize in the docstring. For now I think this is fine but longer term I'm not sure if the console is the correct location for these conditionals maybe better off in one of our settings repos.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm yeah maybe add a write_all function to the settings client that handles this kind of stuff.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@notoriaga notoriaga enabled auto-merge (squash) January 5, 2022 22:18
@notoriaga notoriaga merged commit 97703b0 into main Jan 5, 2022
@notoriaga notoriaga deleted the steve/import-bug branch January 5, 2022 22:43
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