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

Make BrokerUpdater obey file structure #3322

Merged
merged 2 commits into from
Sep 20, 2024

Conversation

THISISDINOSAUR
Copy link
Contributor

Task/Issue URL: https://app.asana.com/0/1203581873609357/1208362583268535/f
Tech Design URL:
CC:

Description:
Not doing this makes it way too easy to accidentally use the fake brokers in prod, which windows has already fallen foul of.

Steps to test this PR:

  1. In the DBP package, in resources, there is one json file at the top level, rename it so fake isn't in the name. I changed fake to fish.
  2. In the actual json file itself, change its name to something you'll recognize
  3. Go into the JSON subdirectory, and change the name of one of those to something you'll recognize (the actual name in the JSON itself)
  4. Start the app
  5. Go to the debug menu, under PIR force a broker update
  6. In the same menu run PIR in debug mode
  7. Look in the list of brokers for the ones you renamed. You SHOULD see the one in the subdirectory, you SHOULDN'T see the one from the top level (so if you named them the same as me, there should be no fish)
  8. You can also use the DB browser to check

Definition of Done:


Internal references:

Pull Request Review Checklist
Software Engineering Expectations
Technical Design Template
Pull Request Documentation

@@ -44,7 +44,7 @@ let package = Package(
.product(name: "Configuration", package: "BrowserServicesKit"),
.product(name: "Persistence", package: "BrowserServicesKit"),
],
resources: [.process("Resources")],
resources: [.copy("Resources")],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

An unfortunate sacrifice to preserve the file structure. Although I don't think it will actually affect anything for now since we don't have e.g. any images in here

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah exactly, I think it’s fine for JSON 👍🏼

Copy link
Contributor

@aataraxiaa aataraxiaa left a comment

Choose a reason for hiding this comment

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

Tested and looks good to me 👍🏼

@THISISDINOSAUR THISISDINOSAUR merged commit 83a67f8 into main Sep 20, 2024
18 checks passed
@THISISDINOSAUR THISISDINOSAUR deleted the elle/change-broker-loading-to-obey-file-system branch September 20, 2024 17:46
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