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

Delete old stuff #467

Merged
merged 13 commits into from
Jan 28, 2025
Merged

Delete old stuff #467

merged 13 commits into from
Jan 28, 2025

Conversation

danielnaab
Copy link
Contributor

@danielnaab danielnaab commented Jan 27, 2025

Remove unused packages.

This also revs the Playwright version, due to errors stemming from rebuilding the lockfile.

Copy link

github-actions bot commented Jan 27, 2025

Terraform plan for tts-10x-forms-dev

Plan: 0 to add, 1 to change, 0 to destroy.
Terraform used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
!~  update in-place

Terraform will perform the following actions:

  # cloudfoundry_app.tts-10x-forms-dev-server-doj_tts-10x-forms-dev-server-doj-app_22D74DC4 will be updated in-place
!~  resource "cloudfoundry_app" "tts-10x-forms-dev-server-doj_tts-10x-forms-dev-server-doj-app_22D74DC4" {
!~      docker_image                    = "ghcr.io/gsa-tts/forms/server-doj:8cb48332a7d7fa856009a2d3d06faeec6e087903" -> "ghcr.io/gsa-tts/forms/server-doj:19be7a968d8f29b9046c7f6224af1920904be9f0"
        id                              = "29c93a0a-519c-454a-847e-cc603395c2da"
        name                            = "tts-10x-forms-dev-server-doj-app"
#        (17 unchanged attributes hidden)

#        (3 unchanged blocks hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.

📝 Plan generated in Post Terraform plan to PR comment #649

@danielnaab danielnaab marked this pull request as ready for review January 27, 2025 23:27
Copy link
Contributor

@kalasgarov kalasgarov left a comment

Choose a reason for hiding this comment

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

The new workspace-dependencies.svg

Looks a little bit too clustered and hard to read.

@danielnaab
Copy link
Contributor Author

The new workspace-dependencies.svg

Looks a little bit too clustered and hard to read.

Yeah, it's been bad for awhile. It used to be in the README and wasn't worth keeping. Maybe we just remove it?

…e prioretize auto-generated docs in the future, we could revisit.
Copy link
Contributor

@kalasgarov kalasgarov left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you for the clean up! 🚀

Copy link
Contributor

@kalasgarov kalasgarov left a comment

Choose a reason for hiding this comment

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

we may also want to clean up this:

if (!dependencyGraphOutputPath) {
dependencyGraphOutputPath = `${workspaceRoot}/workspace-dependencies.svg`;
}
, as it has reference to the svg that removed.

Copy link
Contributor

@ethangardner ethangardner left a comment

Choose a reason for hiding this comment

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

LGTM

ENV PNPM_HOME="/pnpm"
ENV PATH="$PNPM_HOME:$PATH"
ENV NODE_ENV=test
RUN apt-get -y update && apt-get install -y netcat-openbsd
RUN apt-get -y update && apt-get install -y netcat-openbsd make g++
Copy link

@wesley-dean-gsa wesley-dean-gsa Jan 28, 2025

Choose a reason for hiding this comment

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

I ran this through hadolint and it said...

Dockerfile:6 DL3008 warning: Pin versions in apt get install. Instead of `apt-get install <package>` use `apt-get install <package>=<version>`
Dockerfile:6 DL3015 info: Avoid additional packages by specifying `--no-install-recommends`
Dockerfile:6 DL3009 info: Delete the apt-get lists after installing something
Dockerfile:16 DL3016 warning: Pin versions in npm. Instead of `npm install <package>` use `npm install <package>@<version>`
Dockerfile:21 DL3000 error: Use absolute WORKDIR

Just a comment, no need to respond or do anything.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I added a draft issue to track.

@danielnaab danielnaab merged commit 05f4b6d into main Jan 28, 2025
4 checks passed
@danielnaab danielnaab deleted the delete-old-stuff branch January 28, 2025 18:35
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.

4 participants