Skip to content
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

Update install.sh #7973

Merged
merged 3 commits into from
Oct 22, 2024
Merged

Update install.sh #7973

merged 3 commits into from
Oct 22, 2024

Conversation

yadavshubham01
Copy link
Contributor

This PR updates the install.sh script to fetch the docker-compose.yml file from the GitHub branch or tag that matches the version specified by the user, instead of defaulting to the main branch.

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

This PR updates the install.sh script to fetch configuration files based on the specified version, improving version consistency across installations.

  • Modified install.sh to use $version instead of $branch when fetching docker-compose.yml and .env.example
  • Added version detection using GitHub API if not provided as an environment variable
  • Introduced potential backwards compatibility issues for installations using older versions
  • Removed unused $branch variable, which may impact custom installations
  • Consider adding error handling for failed GitHub API requests or file downloads

1 file(s) reviewed, 3 comment(s)
Edit PR Review Bot Settings | Greptile

install.sh Outdated Show resolved Hide resolved
install.sh Outdated Show resolved Hide resolved
@yadavshubham01
Copy link
Contributor Author

@charlesBochet @FelixMalfait please this review this PR

install.sh Outdated

# Replace TAG=latest by TAG=<latest_release or version input>
if [[ $(uname) == "Darwin" ]]; then
# Running on macOS
Copy link
Member

Choose a reason for hiding this comment

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

keep the comments, they are useful here, why remove them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok I will fix it

install.sh Outdated

# Copy twenty/packages/twenty-docker/.env.example to .env
# Set REDIS_URL or REDIS_HOST depending on the version
Copy link
Member

Choose a reason for hiding this comment

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

What is this?

Copy link
Member

Choose a reason for hiding this comment

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

I think this is because of the current bug we have where the docker-compose is using REDIS_URL but 0.31.x images are still using REDIS_HOST

install.sh Outdated
@@ -72,20 +72,18 @@ done
echo "📁 Creating directory '$dir_name'"
mkdir -p "$dir_name" && cd "$dir_name" || { echo "❌ Failed to create/access directory '$dir_name'"; exit 1; }

# Copy the twenty/packages/twenty-docker/docker-compose.yml file in it
# Fetch docker-compose.yml from the branch corresponding to the version or main
Copy link
Member

Choose a reason for hiding this comment

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

Why 'or main'?

Copy link
Member

Choose a reason for hiding this comment

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

I would say same as my comment above

@FelixMalfait FelixMalfait merged commit ec02506 into twentyhq:main Oct 22, 2024
16 checks passed
@twentyhq twentyhq deleted a comment from FelixMalfait Oct 23, 2024
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