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: remove static service IDs #1199

Merged
merged 9 commits into from
Jan 7, 2022

Conversation

ethowitz
Copy link
Contributor

@ethowitz ethowitz commented Jan 3, 2022

Description

Removes the two statically-defined services from the database via a new migration. Makes updates to the application code and integration tests to reflect the change in schema.

Testing

Passing unit and integration tests should sufficiently show that the change was successful.

Issue(s)

Closes #1144

@ethowitz ethowitz requested a review from a team January 3, 2022 18:55
cursor = self.database._execute_sql('DELETE FROM users')
cursor.close
cursor = self.database._execute_sql('DELETE FROM services')
cursor.close()
Copy link
Member

Choose a reason for hiding this comment

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

heh, good catch.

@@ -38,7 +38,7 @@ commands:
command: |
cargo fmt -- --check
# https://github.com/bodil/sized-chunks/issues/11
cargo audit --ignore RUSTSEC-2020-0041 --ignore RUSTSEC-2021-0078 --ignore RUSTSEC-2021-0079 --ignore RUSTSEC-2020-0159 --ignore RUSTSEC-2020-0071 --ignore RUSTSEC-2021-0124
cargo audit --ignore RUSTSEC-2020-0041 --ignore RUSTSEC-2021-0078 --ignore RUSTSEC-2021-0079 --ignore RUSTSEC-2020-0159 --ignore RUSTSEC-2020-0071 --ignore RUSTSEC-2021-0124 --ignore RUSTSEC-2021-0131
Copy link
Member

Choose a reason for hiding this comment

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

Let's put these in a .cargo/audit.toml file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call!

Copy link
Member

@jrconlin jrconlin left a comment

Choose a reason for hiding this comment

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

Looks good. We probably want to note that this is a breaking change, since we're technically no longer offering a service that we didn't really offer before.

@ethowitz
Copy link
Contributor Author

ethowitz commented Jan 5, 2022

@jrconlin Where do you think we should note this? In the code?

@jrconlin
Copy link
Member

jrconlin commented Jan 6, 2022

I think adding it to the commit message and making sure that it's rolled up into the Changelog should be good.
Basically, we're logging the point in time we made the change, should someone need to do discovery later.

@ethowitz ethowitz merged commit ae65970 into master Jan 7, 2022
@ethowitz ethowitz deleted the refactor/1144-remove-static-service-ids branch January 7, 2022 15:41
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.

Remove statically-defined service records
2 participants