You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2022/08/30 13:37:24 UTC
[GitHub] [apisix] jaysonsantos opened a new pull request, #7821: chore: Add devcontainer to ease out for contributors
jaysonsantos opened a new pull request, #7821:
URL: https://github.com/apache/apisix/pull/7821
### Description
Hi there, first thank you very much for this project.
I had a bit of trouble trying to set-up the project for local development and tried to put things in a standardize way to help onboarding new developers.
### Checklist
- [x] I have explained the need for this PR and the problem it solves
- [x] I have explained the changes or the new features added to this PR
- [ ] I have added tests corresponding to this change
- [x] I have updated the documentation to reflect this change
- [x] I have verified that this change is backward compatible (If not, please discuss on the [APISIX mailing list](https://github.com/apache/apisix/tree/master#community) first)
<!--
Note
1. Mark the PR as draft until it's ready to be reviewed.
2. Always add/update tests for any changes unless you have a good reason.
3. Always update the documentation to reflect the changes made in the PR.
4. Make a new commit to resolve conversations instead of `push -f`.
5. To resolve merge conflicts, merge master instead of rebasing.
6. Use "request review" to notify the reviewer after making changes.
7. Only a reviewer can mark a conversation as resolved.
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] jaysonsantos commented on a diff in pull request #7821: chore: Add devcontainer to ease out for contributors
Posted by GitBox <gi...@apache.org>.
jaysonsantos commented on code in PR #7821:
URL: https://github.com/apache/apisix/pull/7821#discussion_r971239020
##########
.devcontainer/Dockerfile:
##########
@@ -0,0 +1,47 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+# See here for image contents: https://github.com/microsoft/vscode-dev-containers/tree/v0.245.0/containers/debian/.devcontainer/base.Dockerfile
+
+# [Choice] Debian version (use bullseye on local arm64/Apple Silicon): bullseye, buster
+ARG VARIANT="buster"
+FROM mcr.microsoft.com/vscode/devcontainers/base:0-${VARIANT}
+
+# Update user's uid based on sent variable
+# It seems it won't work on macos (with podman or rancher desktop) without this
+ARG USERNAME=vscode
+
+COPY .devcontainer/change-uid.sh /usr/local/bin/change-uid.sh
+RUN change-uid.sh
+
+# ** [Optional] Uncomment this section to install additional packages. **
+# RUN apt-get update && export DEBIAN_FRONTEND=noninteractive \
+# && apt-get -y install --no-install-recommends <your-package-list-here>
+RUN apt-get update && apt-get install cpanminus make luajit patch -y
+
+WORKDIR /build
+COPY utils ./utils
+RUN ./utils/install-dependencies.sh
+RUN luarocks install luacheck
+
+RUN mkdir /workspaces && chown vscode: /workspaces
+WORKDIR /workspaces
+USER vscode
+
+RUN mkdir -p /home/vscode/.ssh && ssh-keyscan github.com > /home/vscode/.ssh/known_hosts
+RUN git clone https://github.com/openresty/test-nginx && sudo cpanm ./test-nginx
Review Comment:
Funky because IPC::Run is one of the first dependencies it tries to download.
```
STEP 12/15: RUN git clone https://github.com/openresty/test-nginx && cpanm ./test-nginx
Cloning into 'test-nginx'...
--> Working on ./test-nginx
Configuring /workspaces/test-nginx ... OK
==> Found dependencies: LWP::UserAgent, IPC::Run, Test::Base, Test::LongString, Text::Diff
--> Working on LWP::UserAgent
...
```
not sure why it failed to you, do you still have a log about that?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] jaysonsantos commented on a diff in pull request #7821: chore: Add devcontainer to ease out for contributors
Posted by GitBox <gi...@apache.org>.
jaysonsantos commented on code in PR #7821:
URL: https://github.com/apache/apisix/pull/7821#discussion_r968551822
##########
.devcontainer/Dockerfile:
##########
@@ -0,0 +1,47 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+# See here for image contents: https://github.com/microsoft/vscode-dev-containers/tree/v0.245.0/containers/debian/.devcontainer/base.Dockerfile
+
+# [Choice] Debian version (use bullseye on local arm64/Apple Silicon): bullseye, buster
+ARG VARIANT="buster"
+FROM mcr.microsoft.com/vscode/devcontainers/base:0-${VARIANT}
+
+# Update user's uid based on sent variable
+# It seems it won't work on macos (with podman or rancher desktop) without this
+ARG USERNAME=vscode
+
+COPY .devcontainer/change-uid.sh /usr/local/bin/change-uid.sh
+RUN change-uid.sh
+
+# ** [Optional] Uncomment this section to install additional packages. **
+# RUN apt-get update && export DEBIAN_FRONTEND=noninteractive \
+# && apt-get -y install --no-install-recommends <your-package-list-here>
+RUN apt-get update && apt-get install cpanminus make luajit patch -y
+
+WORKDIR /build
+COPY utils ./utils
+RUN ./utils/install-dependencies.sh
+RUN luarocks install luacheck
+
+RUN mkdir /workspaces && chown vscode: /workspaces
+WORKDIR /workspaces
+USER vscode
+
+RUN mkdir -p /home/vscode/.ssh && ssh-keyscan github.com > /home/vscode/.ssh/known_hosts
+RUN git clone https://github.com/openresty/test-nginx && sudo cpanm ./test-nginx
Review Comment:
hi @soulbird i had a feeling that cpanm already does install the dependencies of the package.
i've managed to run the tests with this same container, did something happen there when you tried?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] soulbird commented on a diff in pull request #7821: chore: Add devcontainer to ease out for contributors
Posted by GitBox <gi...@apache.org>.
soulbird commented on code in PR #7821:
URL: https://github.com/apache/apisix/pull/7821#discussion_r964327399
##########
.devcontainer/Dockerfile:
##########
@@ -0,0 +1,47 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+# See here for image contents: https://github.com/microsoft/vscode-dev-containers/tree/v0.245.0/containers/debian/.devcontainer/base.Dockerfile
+
+# [Choice] Debian version (use bullseye on local arm64/Apple Silicon): bullseye, buster
+ARG VARIANT="buster"
+FROM mcr.microsoft.com/vscode/devcontainers/base:0-${VARIANT}
+
+# Update user's uid based on sent variable
+# It seems it won't work on macos (with podman or rancher desktop) without this
+ARG USERNAME=vscode
+
+COPY .devcontainer/change-uid.sh /usr/local/bin/change-uid.sh
+RUN change-uid.sh
+
+# ** [Optional] Uncomment this section to install additional packages. **
+# RUN apt-get update && export DEBIAN_FRONTEND=noninteractive \
+# && apt-get -y install --no-install-recommends <your-package-list-here>
+RUN apt-get update && apt-get install cpanminus make luajit patch -y
+
+WORKDIR /build
+COPY utils ./utils
+RUN ./utils/install-dependencies.sh
+RUN luarocks install luacheck
+
+RUN mkdir /workspaces && chown vscode: /workspaces
+WORKDIR /workspaces
+USER vscode
+
+RUN mkdir -p /home/vscode/.ssh && ssh-keyscan github.com > /home/vscode/.ssh/known_hosts
+RUN git clone https://github.com/openresty/test-nginx && sudo cpanm ./test-nginx
Review Comment:
Refer to here https://github.com/apache/apisix/blob/master/docs/en/latest/building-apisix.md#building-runtime-for-apisix, you also need to install the dependencies of test-nginx.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] soulbird commented on a diff in pull request #7821: chore: Add devcontainer to ease out for contributors
Posted by GitBox <gi...@apache.org>.
soulbird commented on code in PR #7821:
URL: https://github.com/apache/apisix/pull/7821#discussion_r969065620
##########
.devcontainer/Dockerfile:
##########
@@ -0,0 +1,47 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+# See here for image contents: https://github.com/microsoft/vscode-dev-containers/tree/v0.245.0/containers/debian/.devcontainer/base.Dockerfile
+
+# [Choice] Debian version (use bullseye on local arm64/Apple Silicon): bullseye, buster
+ARG VARIANT="buster"
+FROM mcr.microsoft.com/vscode/devcontainers/base:0-${VARIANT}
+
+# Update user's uid based on sent variable
+# It seems it won't work on macos (with podman or rancher desktop) without this
+ARG USERNAME=vscode
+
+COPY .devcontainer/change-uid.sh /usr/local/bin/change-uid.sh
+RUN change-uid.sh
+
+# ** [Optional] Uncomment this section to install additional packages. **
+# RUN apt-get update && export DEBIAN_FRONTEND=noninteractive \
+# && apt-get -y install --no-install-recommends <your-package-list-here>
+RUN apt-get update && apt-get install cpanminus make luajit patch -y
+
+WORKDIR /build
+COPY utils ./utils
+RUN ./utils/install-dependencies.sh
+RUN luarocks install luacheck
+
+RUN mkdir /workspaces && chown vscode: /workspaces
+WORKDIR /workspaces
+USER vscode
+
+RUN mkdir -p /home/vscode/.ssh && ssh-keyscan github.com > /home/vscode/.ssh/known_hosts
+RUN git clone https://github.com/openresty/test-nginx && sudo cpanm ./test-nginx
Review Comment:
It seems that some dependent packages are not installed. After I use this method, this step can be passed through
```shell
RUN sudo cpanm --notest Test::Nginx IPC::Run
RUN git clone https://github.com/openresty/test-nginx && sudo cpanm ./test-nginx
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tzssangglass commented on pull request #7821: chore: Add devcontainer to ease out for contributors
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on PR #7821:
URL: https://github.com/apache/apisix/pull/7821#issuecomment-1234113470
CI error: https://github.com/apache/apisix/runs/8125739733?check_suite_focus=true
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] jaysonsantos commented on pull request #7821: chore: Add devcontainer to ease out for contributors
Posted by GitBox <gi...@apache.org>.
jaysonsantos commented on PR #7821:
URL: https://github.com/apache/apisix/pull/7821#issuecomment-1232894050
@nic-6443 could you approve again so the markdown linter runs again?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] jaysonsantos commented on pull request #7821: chore: Add devcontainer to ease out for contributors
Posted by GitBox <gi...@apache.org>.
jaysonsantos commented on PR #7821:
URL: https://github.com/apache/apisix/pull/7821#issuecomment-1233299643
@nic-6443 i've added the missing licenses
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] github-actions[bot] commented on pull request #7821: chore: Add devcontainer to ease out for contributors
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #7821:
URL: https://github.com/apache/apisix/pull/7821#issuecomment-1356763125
This pull request/issue has been closed due to lack of activity. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tzssangglass commented on pull request #7821: chore: Add devcontainer to ease out for contributors
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on PR #7821:
URL: https://github.com/apache/apisix/pull/7821#issuecomment-1234581480
> @tzssangglass how do you feel about adding something like [pre-commit](https://pre-commit.com/) to add all the linters in it?
> `make lint` does not cover eclint or the one for markdown
I'm not familiar with this, I remember we had a discussion about pre-commit. ref: https://github.com/apache/apisix/pull/3624
IMO, I think it will increase the maintenance burden. After adding it, we need to at least distinguish which files don't need to be checked.
Anyway, you can open a new issue and let's hear what others think.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] nic-6443 commented on pull request #7821: chore: Add devcontainer to ease out for contributors
Posted by GitBox <gi...@apache.org>.
nic-6443 commented on PR #7821:
URL: https://github.com/apache/apisix/pull/7821#issuecomment-1232998028
> @nic-6443 could you approve again so the markdown linter runs again?
ok.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] soulbird commented on pull request #7821: chore: Add devcontainer to ease out for contributors
Posted by GitBox <gi...@apache.org>.
soulbird commented on PR #7821:
URL: https://github.com/apache/apisix/pull/7821#issuecomment-1238844026
I tried it and it doesn't work under Mac(Intel). I got errors like the following:
````
ERROR: for etcd_tls Cannot start service etcd_tls: Mounts denied:
The path /workspaces/apisix/t/certs is not shared from the host and is not known to Docker.
You can configure shared paths from Docker -> Preferences... -> Resources -> File Sharing.
See https://docs.docker.com/desktop/mac for more info.
````
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on pull request #7821: chore: Add devcontainer to ease out for contributors
Posted by GitBox <gi...@apache.org>.
membphis commented on PR #7821:
URL: https://github.com/apache/apisix/pull/7821#issuecomment-1238987495
The devcontainer is not good way for APISIX developer, it depends on Node.js, Python and C++ compiler, but APISIX do not need them.
Here is the link of devcontainer:
https://code.visualstudio.com/docs/remote/devcontainer-cli#_system-requirements
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] github-actions[bot] commented on pull request #7821: chore: Add devcontainer to ease out for contributors
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #7821:
URL: https://github.com/apache/apisix/pull/7821#issuecomment-1321087215
This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 4 weeks if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the dev@apisix.apache.org list. Thank you for your contributions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] soulbird commented on a diff in pull request #7821: chore: Add devcontainer to ease out for contributors
Posted by GitBox <gi...@apache.org>.
soulbird commented on code in PR #7821:
URL: https://github.com/apache/apisix/pull/7821#discussion_r969065620
##########
.devcontainer/Dockerfile:
##########
@@ -0,0 +1,47 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+# See here for image contents: https://github.com/microsoft/vscode-dev-containers/tree/v0.245.0/containers/debian/.devcontainer/base.Dockerfile
+
+# [Choice] Debian version (use bullseye on local arm64/Apple Silicon): bullseye, buster
+ARG VARIANT="buster"
+FROM mcr.microsoft.com/vscode/devcontainers/base:0-${VARIANT}
+
+# Update user's uid based on sent variable
+# It seems it won't work on macos (with podman or rancher desktop) without this
+ARG USERNAME=vscode
+
+COPY .devcontainer/change-uid.sh /usr/local/bin/change-uid.sh
+RUN change-uid.sh
+
+# ** [Optional] Uncomment this section to install additional packages. **
+# RUN apt-get update && export DEBIAN_FRONTEND=noninteractive \
+# && apt-get -y install --no-install-recommends <your-package-list-here>
+RUN apt-get update && apt-get install cpanminus make luajit patch -y
+
+WORKDIR /build
+COPY utils ./utils
+RUN ./utils/install-dependencies.sh
+RUN luarocks install luacheck
+
+RUN mkdir /workspaces && chown vscode: /workspaces
+WORKDIR /workspaces
+USER vscode
+
+RUN mkdir -p /home/vscode/.ssh && ssh-keyscan github.com > /home/vscode/.ssh/known_hosts
+RUN git clone https://github.com/openresty/test-nginx && sudo cpanm ./test-nginx
Review Comment:
It seems that some dependent packages are not installed. After I use this method, this step can be passed through
```shell
RUN sudo cpanm --notest Test::Nginx IPC::Run
RUN git clone https://github.com/openresty/test-nginx && sudo cpanm ./test-nginx
``
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tzssangglass commented on a diff in pull request #7821: chore: Add devcontainer to ease out for contributors
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on code in PR #7821:
URL: https://github.com/apache/apisix/pull/7821#discussion_r960509511
##########
.devcontainer/change-uid.sh:
##########
@@ -0,0 +1,40 @@
+#!/bin/bash
+
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+set -exo pipefail
+
+function change_uid() {
+ local uid="$1"
+ if [ -z "$uid" ]; then
+ echo "Not changing user id"
+ return 0
+ fi
+ local gid="${2:-$uid}"
+ usermod --uid "$uid" --gid "$gid" "$USERNAME"
+ chown -R "$uid:$gid" "/home/$USERNAME"
+}
+
+function change_gid() {
+ local gid="$1"
+ if [ -z "$gid" ]; then
+ echo "Not changing group id"
+ return 0
+ fi
+ groupmod --gid "$CHANGE_USER_GID" "$USERNAME"
+}
Review Comment:
add a blank line at the end of file
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] jaysonsantos commented on pull request #7821: chore: Add devcontainer to ease out for contributors
Posted by GitBox <gi...@apache.org>.
jaysonsantos commented on PR #7821:
URL: https://github.com/apache/apisix/pull/7821#issuecomment-1234254674
@tzssangglass how do you feel about adding something like [pre-commit](https://pre-commit.com/) to add all the linters in it?
`make lint` does not cover eclint or the one for markdown
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] soulbird commented on a diff in pull request #7821: chore: Add devcontainer to ease out for contributors
Posted by GitBox <gi...@apache.org>.
soulbird commented on code in PR #7821:
URL: https://github.com/apache/apisix/pull/7821#discussion_r971422040
##########
.devcontainer/Dockerfile:
##########
@@ -0,0 +1,47 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+# See here for image contents: https://github.com/microsoft/vscode-dev-containers/tree/v0.245.0/containers/debian/.devcontainer/base.Dockerfile
+
+# [Choice] Debian version (use bullseye on local arm64/Apple Silicon): bullseye, buster
+ARG VARIANT="buster"
+FROM mcr.microsoft.com/vscode/devcontainers/base:0-${VARIANT}
+
+# Update user's uid based on sent variable
+# It seems it won't work on macos (with podman or rancher desktop) without this
+ARG USERNAME=vscode
+
+COPY .devcontainer/change-uid.sh /usr/local/bin/change-uid.sh
+RUN change-uid.sh
+
+# ** [Optional] Uncomment this section to install additional packages. **
+# RUN apt-get update && export DEBIAN_FRONTEND=noninteractive \
+# && apt-get -y install --no-install-recommends <your-package-list-here>
+RUN apt-get update && apt-get install cpanminus make luajit patch -y
+
+WORKDIR /build
+COPY utils ./utils
+RUN ./utils/install-dependencies.sh
+RUN luarocks install luacheck
+
+RUN mkdir /workspaces && chown vscode: /workspaces
+WORKDIR /workspaces
+USER vscode
+
+RUN mkdir -p /home/vscode/.ssh && ssh-keyscan github.com > /home/vscode/.ssh/known_hosts
+RUN git clone https://github.com/openresty/test-nginx && sudo cpanm ./test-nginx
Review Comment:
```shell
#0 72.73 Building and testing HTTP-Daemon-6.14 ... OK
#0 77.57 Successfully installed HTTP-Daemon-6.14
#0 77.62 ! Installing the dependencies failed: Module 'Net::HTTP' is not installed
#0 77.62 ! Bailing out the installation for libwww-perl-6.67.
#0 77.71 ! Installing the dependencies failed: Module 'LWP::UserAgent' is not installed
#0 77.71 ! Bailing out the installation for Test-Nginx-0.30.
#0 77.71 18 distributions installed
------
Dockerfile-with-features:32
--------------------
30 | RUN mkdir -p /home/vscode/.ssh && ssh-keyscan github.com > /home/vscode/.ssh/known_hosts
31 | #RUN sudo cpanm --notest Test::Nginx IPC::Run
32 | >>> RUN git clone https://github.com/openresty/test-nginx && sudo cpanm ./test-nginx
33 | ENV PATH=$PATH:/usr/local/openresty/nginx/sbin
34 |
--------------------
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] jaysonsantos commented on pull request #7821: chore: Add devcontainer to ease out for contributors
Posted by GitBox <gi...@apache.org>.
jaysonsantos commented on PR #7821:
URL: https://github.com/apache/apisix/pull/7821#issuecomment-1243908114
@soulbird
> I tried it and it doesn't work under Mac(Intel).
When does that happen? what `devcontainer build .` returns? Or the problem is after it starts? IIRC, the only time it runs etcd is after create (which probably should be after attach to be honest).
@membphis those dependencies are only for the cli but does not invalidate the use of the dockerfile to install all dependencies which is a bit overwhelming for people that are just starting (in my experience at least).
the good thing is that you can open github codespaces (or vscode) with this repo, and you would have the whole setup done. but if the dependencias are too much and one does not care much about the niceties it add, a simple `docker build -t apisix -f .devcontainer/Dockerfile . && docker run -w $PWD -v $PWD:$PWD -u $(id -u) -g $(id -g) apisix` should drop a shell with some development capabilities out of the box (it still would miss docker in docker for the dependencies).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] github-actions[bot] closed pull request #7821: chore: Add devcontainer to ease out for contributors
Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #7821: chore: Add devcontainer to ease out for contributors
URL: https://github.com/apache/apisix/pull/7821
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org