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

Remove the feature to include golangci-lint since it's already included with ghcr.io/devcontainers/features/go:1 #4506

Merged
merged 3 commits into from
Nov 4, 2024

Conversation

PabloZaiden
Copy link
Contributor

The old repo used for the golangci-lint feature is abandoned and a new working release was created in a new repo.
The current release properly supports ARM-based computers.

This PR Fixes #4505

Copy link
Contributor

@weikanglim weikanglim left a comment

Choose a reason for hiding this comment

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

Hi @PabloZaiden thank you for your contribution!

Can you reference any existing artifacts (issues, discussions, PRs) that describes how ownership is being transferred from https://github.com/guiyomh/features to https://github.com/PabloZaiden/devcontainers-features, since it is a fork? From the changeset here, it looks like the changes made are largely just around release management.

Correct if me if I'm wrong, but the arm64 feature support was actually done as part of guiyomh/features#37?

Based on this initial look, I'm going to wait on understanding more about what conversations has been had with the original author guiyomh in the strategy here before providing an approval. Thanks!

@PabloZaiden
Copy link
Contributor Author

PabloZaiden commented Oct 31, 2024

@weikanglim I've contacted the owner of the original repo to ask him to generate new releases since, after that fix (and several other too), they never generated new releases for the devcontainer features.

I'd gladly revert it back to the original repo if it's being regularly updated, but even when the last commit was from 11-30-2023, the last build used by the devcontainers is from mid 2022.

Until that is properly fixed, this issue is blocking development on ARM based computers. (well... blocking might be too strong of a word. If you're using devcontainers, you need to manually edit the file to remove that feature and avoid committing it.)

@weikanglim weikanglim dismissed their stale review October 31, 2024 18:54

Dismissing my own review earlier. Context has been provided -- changing status to reflect that

@PabloZaiden
Copy link
Contributor Author

One of the checks in the pipeline seems to be stuck? It appears as "pending", but the actual jobs were successful

Copy link
Member

@vhvb1989 vhvb1989 left a comment

Choose a reason for hiding this comment

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

Let's not use forks.
I'd rather move the feature out of the devcontainer and use the docs for telling folks to manually install the golangci-lint.

@PabloZaiden
Copy link
Contributor Author

The contributing guide already points to the installation guide for golangci-lint. Would you rather go the path of closing this PR and opening a new one that just removes the outdated dev container feature then?

@PabloZaiden PabloZaiden changed the title Change golangci-lint feature to an updated features repo Remove the feature to include golangci-lint since it's already included with ghcr.io/devcontainers/features/go:1 Nov 4, 2024
@PabloZaiden
Copy link
Contributor Author

@vhvb1989 I've just realized that the ghcr.io/devcontainers/features/go:1 feature already includes golangci-lint and it works on ARM based Macs out of the box.

I've updated the PR with the extra golangci-lint feature removed.

@vhvb1989
Copy link
Member

vhvb1989 commented Nov 4, 2024

/check-enforcer override

@vhvb1989 vhvb1989 merged commit 94c5489 into Azure:main Nov 4, 2024
3 checks passed
@vhvb1989
Copy link
Member

vhvb1989 commented Nov 4, 2024

Thank you @PabloZaiden !

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.

[Issue] Opening the solution in a Devcontainer fails on an ARM-based Mac
3 participants