-
Notifications
You must be signed in to change notification settings - Fork 4
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
test!: Don't validate salesforce orgs against orgs.yml #15
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good change. Should we remove the file? Is there a change log?
BREAKING CHANGE: We no longer test that the "Account Name" for the CSV has a valid entry in the orgs.yaml file in the same directory. This doesn't make sense because salesforce is now the source of truth so also needing we can trust it to have valid orgs. And if it doesn't that should be fixed in the salesforce process and not here.
ac9f88d
to
3e71265
Compare
The org.yml file is still being used by openedx-webooks in some way so while I think it makes sense to remove it in the long term, I think the task is a bit complex so I'm not gonna do that as a part of this change. |
Do we have a plan to put the information in salesforce and update openedx-webhooks to use it? We've removed some validation before replacing it, it seems. |
The information is already in Salesforce, the org is included in the exported file, but we're saying we don't need an additional external validation because Salesforce is the source-of-truth. Is there a use case for another system needing the list of orgs? |
That would be a good thing to find out before a breaking change in the validation :) It's perhaps not needed now, but the OSPR bot reads orgs.yaml to determine internal/external/contractor. |
I honestly think having the CLA check file updated reliably is more important. This accomplishes that. Happy to fix forward if necessary. |
@nedbat so it sounds like we need to update openedx-webhooks bot to read orgs from the CSV as well? I'll add a ticket for that. |
BREAKING CHANGE: We no longer test that the "Account Name" for the CSV has a valid
entry in the orgs.yaml file in the same directory. This doesn't make sense because
salesforce is now the source of truth so also needing we can trust it to have valid
orgs. And if it doesn't that should be fixed in the salesforce process and not here.