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

Improved dev environment #2211

Merged
merged 14 commits into from
Jan 20, 2024

Conversation

kvanzuijlen
Copy link
Member

Description

Adds a devcontainer to make setting up a proper development environment quicker

Motivation and Context

It makes sure tests and linting have been run before a commit is accepted.

How Has This Been Tested?

Tested it locally, saw the pre-commit hooks working

Checklist:

  • My change requires a change to the documentation or CHANGELOG.
  • I have updated the documentation/CHANGELOG accordingly.
  • I have created a feature (non-master) branch for my PR.

Copy link
Member

@tuunit tuunit left a comment

Choose a reason for hiding this comment

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

Revert test removal

Dockerfile.devcontainer Outdated Show resolved Hide resolved
.devcontainer/devcontainer.json Show resolved Hide resolved
Dockerfile.devcontainer Outdated Show resolved Hide resolved
Dockerfile.devcontainer Outdated Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
@kvanzuijlen kvanzuijlen marked this pull request as ready for review September 20, 2023 19:59
@kvanzuijlen kvanzuijlen requested a review from a team as a code owner September 20, 2023 19:59
@kvanzuijlen kvanzuijlen marked this pull request as draft October 7, 2023 08:11
@kvanzuijlen kvanzuijlen marked this pull request as ready for review October 25, 2023 20:43
.devcontainer/devcontainer.json Outdated Show resolved Hide resolved
.vscode/tasks.json Show resolved Hide resolved
pkg/http/server_test.go Outdated Show resolved Hide resolved
.vscode/tasks.json Show resolved Hide resolved
Co-authored-by: Jan Larwig <jan@larwig.com>
@tuunit tuunit added the LGTM PR is ready to merge label Oct 31, 2023
.devcontainer/Dockerfile Outdated Show resolved Hide resolved
@@ -0,0 +1,15 @@
repos:
Copy link
Member

Choose a reason for hiding this comment

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

This will only run in the dev container?

Copy link
Member Author

Choose a reason for hiding this comment

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

Only when pre-commit install has been ran. (postCreateCommand in devcontainer.json)

Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to store all of the relevant code in the .devcontainer folder or must this live in the root?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think this should be in the .devcontainer as it's not devcontainer-specific although I think it should be possible

Copy link
Member

Choose a reason for hiding this comment

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

What else would it be used for?

Copy link
Member Author

Choose a reason for hiding this comment

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

It can also be used for anyone who wants to use pre-commit (which is possible without using the devcontainer)

@@ -0,0 +1,36 @@
{
Copy link
Member

Choose a reason for hiding this comment

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

Is having this checked in likely to cause any conflict with our dev's existing configurations?

Copy link
Member Author

Choose a reason for hiding this comment

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

Potentially, although the launch.json is generally advised to be added to SCM. Locally, this can be overriden using workspace settings.

Copy link
Member

Choose a reason for hiding this comment

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

Can you point to where it is advised to check it in, I've personally not worked on a project where I've seen this checked in

Copy link
Member Author

Choose a reason for hiding this comment

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

I myself have not worked with it a lot either, but it's done by home-assistant/core as well, from which I've largely copied this setup from. I liked it there, which is why I added it, but I'm fine with it gone as well.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, lets keep it for now and we'll see how we get on

Copy link
Contributor

github-actions bot commented Jan 7, 2024

This pull request has been inactive for 60 days. If the pull request is still relevant please comment to re-activate the pull request. If no action is taken within 7 days, the pull request will be marked closed.

@github-actions github-actions bot added the Stale label Jan 7, 2024
@github-actions github-actions bot closed this Jan 14, 2024
@tuunit tuunit reopened this Jan 14, 2024
@tuunit tuunit removed the Stale label Jan 14, 2024
@@ -0,0 +1,15 @@
repos:
Copy link
Member

Choose a reason for hiding this comment

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

What else would it be used for?

@@ -0,0 +1,36 @@
{
Copy link
Member

Choose a reason for hiding this comment

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

Can you point to where it is advised to check it in, I've personally not worked on a project where I've seen this checked in

.pre-commit-config.yaml Outdated Show resolved Hide resolved
.pre-commit-config.yaml Outdated Show resolved Hide resolved
@JoelSpeed JoelSpeed merged commit f88f09f into oauth2-proxy:master Jan 20, 2024
6 checks passed
@kvanzuijlen kvanzuijlen deleted the improved-dev-environment branch January 20, 2024 21:41
tuunit added a commit to tuunit/oauth2-proxy that referenced this pull request Jan 21, 2024
* enhancement: Change base image from alpine to distroless (oauth2-proxy#2295)

* Changed base image from alpine to distroless

* chore: updated Makefile

* fix: removed arm/v6 and ppc64le for distroless variant

* Update Dockerfile

* Update Makefile

* docs: Add README-section, CHANGELOG-entry and --pull to prevent caching

---------

Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk>

* Add possibility to encode the state param as UrlEncodedBase64 (oauth2-proxy#2312)

* Add possibility to encode the state param as UrlEncodedBase64

* Update CHANGELOG.md

* Update oauthproxy.go

Co-authored-by: Jan Larwig <jan@larwig.com>

---------

Co-authored-by: Jan Larwig <jan@larwig.com>

* NGINX return 403 for sign_in (oauth2-proxy#2322) (oauth2-proxy#2323)

Co-authored-by: Sven Ertel <sven.ertel@bayernwerk.de>

* chore: Create sha256sum for tar instead of binary (oauth2-proxy#2343)

* Create sha256sum for tar instead of binary

* chore: Add checksum for binary

* chore: Updated changelog

---------

Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk>

* Log error details when failed loading CSRF cookie (oauth2-proxy#2345)

* Log error details when failed loading CSRF cookie

* Add a record about this PR to CHANGELOG.md

---------

Co-authored-by: Ondrej Charvat <ondrej.charvat@yunextraffic.com>
Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk>

* Feature - Add env variable support for alpha struct (oauth2-proxy#2375)

* added envsubstring package and added simple test cases.imple tests.

* added documentation

* added changelog entry

* added documentation to wrong file


.

* changed tests to ginkgo format

* update project to use better maintained library

* use defer to clear test variable after tests finished

* updated docs for the new package documentation and fixed bad english

* refactored function to "reduce" complexity.

* updated changelog for new version

updated readme

* minor formatting

---------

Co-authored-by: Haydn Evans <h.evans@douglas.de>
Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk>

* remove nsswitch workaround (oauth2-proxy#2371)

Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk>

* feat: Added renovate configuration (oauth2-proxy#2377)

* Feature/add option to skip loading claims from profile url (oauth2-proxy#2329)

* add new flag skip-claims-from-profile-url

* skip passing profile URL if SkipClaimsFromProfileURL

* docs for --skip-claims-from-profile-url flag

* update flag comment

* update docs

* update CHANGELOG.md

* Update providers/provider_data.go

Co-authored-by: Jan Larwig <jan@larwig.com>

* Add tests for SkipClaimsFromProfileURL

* simplify tests for SkipClaimsFromProfileURL

* generate alpha_config.md

---------

Co-authored-by: Jan Larwig <jan@larwig.com>

* Add ability to configure username for Redis cluster connections (oauth2-proxy#2381)

* Initial attempt.

* Add CHANGELOG entry.

* Drop commented-out Sentinel test.

---------

Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk>

* chore(deps): update module golang.org/x/crypto to v0.17.0 [security] (oauth2-proxy#2400)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* chore(deps): update github.com/ghodss/yaml digest to d8423dc (oauth2-proxy#2401)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* Improved dev environment (oauth2-proxy#2211)

* Improved dev env setup

* Cleanup duplicate checks

* Applied PR feedback

* Updated go.mod/go.sum

* go mod tidy

* Update .devcontainer/devcontainer.json

* Update pkg/http/server_test.go

Co-authored-by: Jan Larwig <jan@larwig.com>

* Create launch.json

* Update .devcontainer/Dockerfile

* Apply suggestions from code review

---------

Co-authored-by: Jan Larwig <jan@larwig.com>

* feature: add release automation workflows (oauth2-proxy#2224)

* feature: add release automation workflows

* deactivate provenancee because of behaviour change with buildx v0.10.0

* add changelog section extraction for github release notes

* fix registry path; fix EOF

* use correct version of golangci-lint; add additional workflow step for fetching all dependencies

* chore(deps): update module github.com/bsm/redislock to v0.9.4 (oauth2-proxy#2406)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* initial commit for docusaurus 3 upgrade

* fix mdx errors

* fix mdx issues

* fix routing issues

* update docs generation workflow

* fix version

* fix permissions

* move slack to header

* remove background color and minify

* Update docs/docs/configuration/providers/openid_connect.md

Co-authored-by: Koen van Zuijlen <8818390+kvanzuijlen@users.noreply.github.com>

* Update docs/docs/configuration/providers/openid_connect.md

Co-authored-by: Koen van Zuijlen <8818390+kvanzuijlen@users.noreply.github.com>

* Update docs/docs/configuration/providers/openid_connect.md

Co-authored-by: Koen van Zuijlen <8818390+kvanzuijlen@users.noreply.github.com>

* Update docs/docs/configuration/providers/gitlab.md

Co-authored-by: Koen van Zuijlen <8818390+kvanzuijlen@users.noreply.github.com>

* Update docs/docs/configuration/providers/gitlab.md

Co-authored-by: Koen van Zuijlen <8818390+kvanzuijlen@users.noreply.github.com>

* Update docs/docs/configuration/providers/github.md

Co-authored-by: Koen van Zuijlen <8818390+kvanzuijlen@users.noreply.github.com>

* Update docs/docs/configuration/providers/github.md

Co-authored-by: Koen van Zuijlen <8818390+kvanzuijlen@users.noreply.github.com>

* Update docs/docs/configuration/providers/github.md

Co-authored-by: Koen van Zuijlen <8818390+kvanzuijlen@users.noreply.github.com>

* Update docs/docs/configuration/providers/github.md

Co-authored-by: Koen van Zuijlen <8818390+kvanzuijlen@users.noreply.github.com>

---------

Co-authored-by: Koen van Zuijlen <8818390+kvanzuijlen@users.noreply.github.com>
Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk>
Co-authored-by: Jan Brezina <brezinajn@users.noreply.github.com>
Co-authored-by: WhiteRabbit-Code <sven@ertel-net.de>
Co-authored-by: Sven Ertel <sven.ertel@bayernwerk.de>
Co-authored-by: charvadzo <120425386+charvadzo@users.noreply.github.com>
Co-authored-by: Ondrej Charvat <ondrej.charvat@yunextraffic.com>
Co-authored-by: Haydn Evans <h.evans@douglas.de>
Co-authored-by: Nils Gustav Stråbø <65334626+nilsgstrabo@users.noreply.github.com>
Co-authored-by: Ross Golder <ross@golder.org>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docker go LGTM PR is ready to merge tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants