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

Do not validate commit ID from within projectfile. #3490

Merged
merged 1 commit into from
Sep 16, 2024

Conversation

mitchell-as
Copy link
Contributor

@mitchell-as mitchell-as commented Sep 16, 2024

BugDX-3050 The attempt to fix invalid URL by `state reset` fail

Doing so prevents state reset from fixing a bad ID.

Any runners that need a commit ID get it from localcommit, which raises an invalid commit ID error, so this validation is superfluous.

Doing so prevents `state reset` from fixing a bad ID.

Any runners that need a commit ID get it from localcommit, which raises an invalid commit ID error, so this validation is superfluous.
@mitchell-as mitchell-as requested a review from Naatan September 16, 2024 18:51
@mitchell-as mitchell-as marked this pull request as ready for review September 16, 2024 18:51
Copy link
Member

@Naatan Naatan left a comment

Choose a reason for hiding this comment

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

I don't like removing functionality to fix a bug. But I also see little value in the functionality being removed here. It wasn't a very friendly error for the user, and from what I can tell through the blame log this was introduced for headless commits, which we no longer have.

So accepting this one but beware of fixing bugs this way, more often than not this'd be an inappropriate fix.

@mitchell-as
Copy link
Contributor Author

I agree. I tried to fix this properly, but ran into a mountain of issues because project parsing was occurring so early in main.go. After going back to the drawing board, I came up with this solution.

@mitchell-as mitchell-as merged commit 56a1cb4 into version/0-46-0-RC1 Sep 16, 2024
8 checks passed
@mitchell-as mitchell-as deleted the mitchell/dx-3050 branch September 16, 2024 19:54
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