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

Apply the cookiecutter to h #9267

Merged
merged 1 commit into from
Jan 23, 2025
Merged

Apply the cookiecutter to h #9267

merged 1 commit into from
Jan 23, 2025

Conversation

seanh
Copy link
Contributor

@seanh seanh commented Jan 22, 2025

Apply the templates from https://github.com/hypothesis/cookiecutters/tree/main/pyramid-app to h.

Known issues:

  • The websocket process isn't working in make docker-run. But it's broken on main as well.

  • make and tox commands are printing lots of Python warnings at startup, see: Remove PYTHONDEVMODE cookiecutters#211

  • The make web convenience command is broken, but this is a cookiecutter issue that also affects other projects: make web is broken cookiecutters#212

  • We've lost some h-specific files from deploy.yml's paths-ignore setting (which is the list of files that won't trigger an automatic deploy to staging). The cookiecutter doesn't have an include for extending this list. In the future we intend to move away from automatic deploys on merge anyway so there's no point in fixing this.

  • The cookiecutter adds mypy to all projects (I don't think there's an option to disable it?). For now I've just put this in pyproject.toml to effectively disable type-checking even though there is a make typecheck and a type-checking job on CI. We can enable it as a separate PR.

    [[tool.mypy.overrides]]
    module = [
      # Don't try to typecheck the tests for now.
      "tests.*",
      # Disable type-checking in h (by ignoring all files) for now.
      # This is just to get `make typecheck` passing.
      # In future we should remove this and fix h's type-checking errors.
      "h.*",
    ]
    ignore_errors = true

Merging:

  • I think the Docker image build is working, but we won't know for sure until we merge this and try to build a release, push it to Docker Hub, and deploy it to staging.
  • We should test that DB migrations are still working in staging and prod.
  • Also test that the websocket is working on staging, Canada and production

@seanh seanh changed the title apply cookiecutter Apply the cookiecutter to h Jan 22, 2025
@seanh seanh force-pushed the apply-cookiecutter branch 6 times, most recently from 1a6e18b to f57716e Compare January 23, 2025 11:34
@seanh seanh force-pushed the apply-cookiecutter branch from f57716e to f0d285d Compare January 23, 2025 11:45
@seanh seanh marked this pull request as ready for review January 23, 2025 11:48
@seanh seanh requested a review from marcospri January 23, 2025 12:23
Copy link
Member

@marcospri marcospri left a comment

Choose a reason for hiding this comment

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

I haven't done a line by line review but I don't see anything that seems odd.

Locally I sanity checked basic functionally and some dev commands I use often (make sql|shell|format|lint|test|coverage)

Looks good 👍

@seanh seanh merged commit eb998e1 into main Jan 23, 2025
11 checks passed
@seanh seanh deleted the apply-cookiecutter branch January 23, 2025 16:32
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.

2 participants