Skip to content

Update the build command for OLC #122

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.

Sign up for GitHub

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

Open
marekweb wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Update the build command for OLC #122

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c13b5aa
Update the build command for OLC
marekweb Jul 10, 2024
8fbdd71
Update OLC build command in setup
marekweb Jul 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion setup.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ install_hooks() {

install_our_lovely_cli() {
cd "$DEVTOOLS_DIR/our-lovely-cli"
npm install
make
Copy link
Member

@csilvers csilvers Jul 11, 2024

Choose a reason for hiding this comment

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

On second thought, we should have an explicit make target here, rather than just depending on the first one being the one to install. I'm not even sure what it should be, looking at the (confusing) Makefile. make yarn? make all? You may want to ask in #github-prs.

Copy link

@lillialexis lillialexis Jul 17, 2024

Choose a reason for hiding this comment

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

@csilvers we're having problems with our Makefile, and it's been on my list to reach out to you to chat w you (when you have time), as everyone says you're the "make expert" here. I wonder if we should solve that first, since the solution could be to just get rid of the Makefile altogether...

Copy link
Member

@csilvers csilvers Jul 17, 2024

Choose a reason for hiding this comment

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

@lillialexis am happy to chat with you! My calendar is accurate if you want to find time tomorrow.

Copy link

@lillialexis lillialexis Jul 18, 2024

Choose a reason for hiding this comment

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

@marekweb please use make all

Copy link

@lillialexis lillialexis Jul 22, 2024

Choose a reason for hiding this comment

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

oh, wait, maybe make all doesn't do yarn build, but I'll fix that in OLC momentarily...

Copy link
Author

@marekweb marekweb Jul 23, 2024

Choose a reason for hiding this comment

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

Alright, I updated it to make all assuming that will be the final command.

}

check_dependencies
Expand Down