-
-
Notifications
You must be signed in to change notification settings - Fork 777
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
1255 refactor current data into data external #1267
1255 refactor current data into data external #1267
Conversation
Looks OK at a glance. Does someone else have time to review it properly? |
And I think .github/workflows/_wins-data.yml and .github/workflows/wins-data.yml are 2 versions of the same thing. Suspect only one is in use. Could someone check and delete the one that is no longer in use? In this reorg or in a separate PR. |
@cnk They are not the same file one is using curl to pull data, one is using node script to pull data. You are right that currently only one of them is in use. The _wins-data.yml is a work in progress that would eventually be used instead of the current wins-data.yml will be deleted and the _wins-data.yml will be renamed to wins-data.yml |
@cnk What would be the best way to review this? |
@jbubar For testing substantial changes to GitHub actions, I usually change the triggering conditions so I can run them at will - and then run them in my fork. But for the changes here, simply changing the directory where the data is written, I think it is sufficient to proofread the files to make sure we caught all places that need to be changed to match. |
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.
Looks good. I proof read it very closely, but I did not test it out. I think it should be good
Looked over it a few more times, and merged it |
Fixes #1266
Please looks at the commit messages for reference to what this pr changes