Skip to content

Conversation

@viceice
Copy link
Member

@viceice viceice commented Jul 18, 2023

We now call back into our new cli to download the file, so the new url replacement can be applied.
Will add some more documentation too later.

@viceice viceice requested review from Chumper and rarkins July 18, 2023 08:17
@Chumper
Copy link
Contributor

Chumper commented Jul 18, 2023

We can phase out the bats tests whenever we replace functionality with typescript

@viceice
Copy link
Member Author

viceice commented Jul 18, 2023

We can phase out the bats tests whenever we replace functionality with typescript

fixed the tests. I'm not sure if i need the file delete 🤔

@viceice
Copy link
Member Author

viceice commented Jul 18, 2023

We can phase out the bats tests whenever we replace functionality with typescript

fixed the tests. I'm not sure if i need the file delete 🤔

https://stackoverflow.com/questions/37176783/node-js-fs-more-efficient-way-to-create-file-clear-file

fs.createWriteStream will truncate exisitng file

@viceice viceice marked this pull request as ready for review July 18, 2023 10:06
@viceice viceice requested a review from Chumper July 18, 2023 11:47
Copy link
Contributor

@rarkins rarkins 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 get docs for the URL rewriting if this closes the issue?

@viceice
Copy link
Member Author

viceice commented Jul 18, 2023

Can we get docs for the URL rewriting if this closes the issue?

there are already some docs and all urls are printed on debug log level

@viceice
Copy link
Member Author

viceice commented Jul 19, 2023

P My

???

@rarkins
Copy link
Contributor

rarkins commented Jul 20, 2023

Not sure where that came from. Can you point me to the docs?

@viceice
Copy link
Member Author

viceice commented Jul 20, 2023

Not sure where that came from. Can you point me to the docs?

https://github.com/containerbase/base/tree/feat/url-replace#url-replacement

will create a mother extended version in later PR's.

will try to extract the base urls to variables, so I can use a test to validate docs to have a sample for each of them.

Copy link
Contributor

@rarkins rarkins left a comment

Choose a reason for hiding this comment

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

Don't want to block this but we need conclusive docs for rewriting all URLs in a "full" image

@viceice
Copy link
Member Author

viceice commented Jul 22, 2023

yes, will update docs later. currently it's best to try and error to see failed urls in logs. then it should be easy to rewrite them.

@viceice viceice added this pull request to the merge queue Jul 22, 2023
Merged via the queue into main with commit cdec808 Jul 22, 2023
@viceice viceice deleted the feat/url-replace branch July 22, 2023 10:53
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.

Allow all external download URLs to be aliased

4 participants