Corrected the sed -i command in script/package shell script [#554] by bazzaar · Pull Request #672 · github/training-kit · GitHub
Skip to content

Corrected the sed -i command in script/package shell script [#554]#672

Merged
brntbeer merged 2 commits into
github:masterfrom
bazzaar:bazzaar_script_fix
Apr 4, 2019
Merged

Corrected the sed -i command in script/package shell script [#554]#672
brntbeer merged 2 commits into
github:masterfrom
bazzaar:bazzaar_script_fix

Conversation

@bazzaar

@bazzaar bazzaar commented Mar 31, 2019

Copy link
Copy Markdown
Contributor

Overview

In the training-kit README, a workflow is presented to create a local copy of the files to be served by a web-server. The first step of that workflow is to run script/package, but that step fails ...

This PR fixes two errors in the sed command inside script/package [ see comment by bazzaar in issue #554 ]

I subsequently ran through the corrected workflow on my local version of training-kit, and the web-served files are built and packaged successfully, and then display correctly in Firefox browser using the simple python web server.

Questions

As the sed regex didn't match the text in the _config.yml, perhaps some change had been made to the _config.yml file previously, ... maybe the keywords are no longer as expected in that file? Just a thought ...

hope this helps
bazzaar

@bazzaar

bazzaar commented Mar 31, 2019

Copy link
Copy Markdown
Contributor Author

@brianamarie

Copy link
Copy Markdown
Contributor

@brntbeer I seem to remember you creating the packaging script - would you have time to review this PR? 👀

@brntbeer

brntbeer commented Apr 2, 2019

Copy link
Copy Markdown
Member

@bazzaar first off, welcome! thank you for this contribution.

Sorry for the delay in looking into this, I'll take a look now ! 🎉

Comment thread script/package Outdated
@bazzaar

bazzaar commented Apr 2, 2019 via email

Copy link
Copy Markdown
Contributor Author

Comment thread script/package Outdated
@brntbeer

brntbeer commented Apr 2, 2019

Copy link
Copy Markdown
Member

Did you see my comment in the PR about the possible redundant kit subdir?

Yep, you're right. I'm taking a look at that but would prefer to address that after this PR is merged if you dont mind. that way we can have these two changes be atomic

Co-Authored-By: bazzaar <barryfoxbat@btinternet.com>
@brntbeer

brntbeer commented Apr 4, 2019

Copy link
Copy Markdown
Member

@brntbeer brntbeer merged commit 390c9f8 into github:master Apr 4, 2019
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.

3 participants