Skip to content

fix url error#5

Open
chiarng wants to merge 3 commits into
mainfrom
urlerror
Open

fix url error#5
chiarng wants to merge 3 commits into
mainfrom
urlerror

Conversation

@chiarng
Copy link
Copy Markdown
Member

@chiarng chiarng commented Aug 10, 2021

No description provided.

@chiarng
Copy link
Copy Markdown
Member Author

chiarng commented Aug 11, 2021

i'm trying to understand the resultsUrl change that was made from another GitHub Action:

https://github.com/ServiceNow/sncicd-publish-app/blob/bf1e648b24b1abcc948b5b802e9d8342b8ae8096/src/App.ts#L155-L160

Comment thread src/App.ts
const response: Responce = await axios.get(result.links.progress.url, this.config)
resultsUrl = result.links.results.url
const response: Response = await axios.get(result.links.progress.url, this.config)
resultsUrl = result.links.results?.url
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actually I think we need to put this not in here since this is simply printing out the status until it is successful.

Comment thread src/App.ts
resultsUrl = result.links.results?.url
// Throttling
await this.sleep(this.sleepTime)
// Call itself if the request in the running or pending state
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Also this else block is said for testing only... Not sure what that means especially since it will be actually called. I am rather confused by the purpose of this method and why it was implemented this way.

@bmeeder22
Copy link
Copy Markdown

Do you not have github acations setup for this to automatically run our unit tests?

@chiarng
Copy link
Copy Markdown
Member Author

chiarng commented Aug 12, 2021

Hah. Ironically, no, been just running the unit tests locally. I suppose it wouldn't be that hard to set up an Actions Workflow for a simple npm run test, and would make a lot more sense for the open-source thing.

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.

"url" property and "installInstance" input expected by the batch-install action

2 participants