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

Fix region seeder name #1233

Closed
wants to merge 2 commits into from
Closed

Fix region seeder name #1233

wants to merge 2 commits into from

Conversation

jonathangoulding
Copy link
Collaborator

A naught fella has been left in the code. This should be removed

A naught fella has been left in the code. This should be removed
@jonathangoulding jonathangoulding self-assigned this Aug 5, 2024
@jonathangoulding jonathangoulding added the bug Something isn't working label Aug 5, 2024
@Cruikshanks
Copy link
Member

Unless it is urgent, this was spotted and will be resolved by #1230

@jonathangoulding
Copy link
Collaborator Author

Unless it is urgent, this was spotted and will be resolved by #1230

This should be breaking tests - but i think they are testing the display name and not the name. So maybe not urgent if the next work is going in.

Copy link
Contributor

@Beckyrose200 Beckyrose200 left a comment

Choose a reason for hiding this comment

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

Can we change it for the test region, so it sets the isTest column to true? The tear-down service for the acceptance tests utilises the test region isTest property to know what data to delete and with the seeder test region the tear-down is breaking

@jonathangoulding
Copy link
Collaborator Author

Can we change it for the test region, so it sets the isTest column to true? The tear-down service for the acceptance tests utilises the test region isTest property to know what data to delete and with the seeder test region the tear-down is breaking

You would need to update the data file and columns. Maybe set the default to false allow the data file to override. Might be worth a check on the PR mentioned if that's considered this.

@Cruikshanks Cruikshanks added the do not merge Used for spikes and experiments label Aug 5, 2024
@Cruikshanks
Copy link
Member

Parking this as we have #1230 incoming.

It also deals with @Beckyrose200 question about flagging the region as is_test.

@Cruikshanks
Copy link
Member

Closing as covered by #1230

@Cruikshanks Cruikshanks closed this Aug 7, 2024
@Cruikshanks Cruikshanks deleted the remove-fella branch August 7, 2024 23:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working do not merge Used for spikes and experiments
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants