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

Add a workflow to build docker image #202

Merged
merged 12 commits into from
Dec 6, 2023
32 changes: 31 additions & 1 deletion .github/workflows/pipeline.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,37 @@ jobs:
- uses: actions/setup-node@v3
with:
node-version: 18
# TODO build:web fails currently, but it'd be good to make that work when possible and then add it here.
- run: yarn
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is just a drive-by cleanup now that build:web is sorted out.

Copy link
Contributor Author

@H-Shay H-Shay Dec 6, 2023

Choose a reason for hiding this comment

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

I'm moving this to a separate PR so that building the docker images is not stuck behind getting access to the repo controls and changing the required checks. The changes have been moved to here: #204

- run: yarn build:jsonschema
- run: yarn build:ts

build-docker:
name: Build Docker Image
runs-on: ubuntu-latest
env:
# Only push if this is develop, otherwise we just want to build
# On a PR github.ref is the target branch, so don't push for that either
PUSH: ${{ github.ref == 'refs/heads/master' && github.event_name != 'pull_request' }}

steps:
- name: Check out
uses: actions/checkout@v2

- name: Login to GHCR
uses: docker/login-action@v3
if: ${{ env.PUSH == 'true' }}
with:
registry: ghcr.io
username: ${{ github.actor }}
password: ${{ secrets.GITHUB_TOKEN }}

- name: Build image
uses: docker/build-push-action@v2
with:
context: .
file: ./Dockerfile
platforms: ${{ env.PLATFORMS }}
push: ${{ env.PUSH }}
tags: |
ghcr.io/matrix-org/conference-bot:latest
ghcr.io/matrix-org/conference-bot:${{ github.sha }}
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@
"moment": "^2.29.4",
"node-fetch": "^2.6.1",
"pg": "^8.9.0",
"postcss-preset-env": "^6.7.0",
"querystring-es3": "^0.2.1",
"prom-client": "^15.0.0",
"qs": "^6.11.2",
"rfc4648": "^1.4.0",
Expand Down
2 changes: 1 addition & 1 deletion src/web.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ limitations under the License.
import { Response, Request } from "express";
import template from "./utils/template"
import config from "./config";
import { base32 } from "rfc4648";
const { base32 } = require('rfc4648');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't pretend to understand javascript modules but I hit a compilation error here after merging in main and this fixed it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm going to quickly check this one. Technically this drops types so we might introduce an error if the function is undefined or something.

import { LogService, MatrixClient } from "matrix-bot-sdk";
import { sha256 } from "./utils";
import * as dns from "dns";
Expand Down
1 change: 1 addition & 0 deletions tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"compilerOptions": {
"experimentalDecorators": true,
"emitDecoratorMetadata": true,
"lib": ["DOM", "es2022"],
H-Shay marked this conversation as resolved.
Show resolved Hide resolved
"noImplicitAny": false,
"strict": true,
"strictPropertyInitialization": false,
Expand Down
1 change: 1 addition & 0 deletions webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ module.exports = {
},
resolve: {
extensions: ['.ts', '.js'],
fallback: { "querystring": require.resolve("querystring-es3") }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Apparently querystring has been deprecated for awhile? It was causing errors failing to resolve and this is the answer my internet googling turned up.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the proper fix is to use URLSearchParams where you can, as it's native to the browser. Not worth fixing in this PR but we should tidy up our frontend code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll look at fixing it in a separate PR, thanks for giving the correct fix!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(hopefully) fixed in #205

},
plugins: [
new CleanWebpackPlugin({ cleanStaleWebpackAssets: false }),
Expand Down
Loading
Loading