-
Notifications
You must be signed in to change notification settings - Fork 24
Use actions workflow parser for auto settings #175
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
base: main
Are you sure you want to change the base?
Use actions workflow parser for auto settings #175
Conversation
1b0fbc6 to
3ccaf89
Compare
|
Note seems like every expression with errors are skipped without showing errors, so in that point of view no new behavior. if: vars['OPEN_VSX_TOKEN8'] |
|
Possible should get a deep performance analysis due to #174 The problem in this impl is that each workflow is parsed more than once |
|
@ChristopherHX Thanks for taking a look at this. Could you resolve the merge conflict as a couple PRs I had got merged in. I will review this PR in the coming days. By chance, did you follow some reference implementation/documentation? If yes, can you point me to that code as well? It will help me better understand this implementation.
The main performance problem here is that the extension is constantly writing and reading from the extensions |
…l-actions into use-actions-workflow-parser-for-auto-settings
I observed as soon you enable / disable a secret/setting the whole treeview reads all workflow files gain, this might also not be nothing. (for the manual refresh this is ok) |
No, I have written this from scratch and eventually looked into the source code as well. I knew there must be a traverse function of the raw parsing result from c#, while the once I use is from 2019 it looks pretty similar with the 2023 js version. I'm working with the c# codebase of these packages since a long time e.g. from the c# side I already had written such a function, which seems to have far less lines of code |
|
@ChristopherHX FYI, due to the performance issues with how settings are stored, I am planning to rework that before having a look at this as those changes might impact what is done here. I have been a bit busy lately, but will try to get something soon. |
|
No problem, I'm busy as well with my other projects. My primary goal was to show you, what could be done by the official Actions npm packages. I know, I'm writing 2 weeks later xd |
✍ Changes
Proof of concept, feel free to change this as needed
Now both get parsed and added to settings pane
Inside the index visitor, I didn't liked the visitor pattern anymore and used instanceOf instead of creating another visitor
📋 Checklist
Closes #163