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

Clarified MF_TRANSSHIFT purpose comment in Doom #1091

Merged
merged 1 commit into from
Aug 25, 2023

Conversation

SoDOOManiac
Copy link
Collaborator

For Heretic, Hexen and Strife the meaningful comments are present.

For other games the meaningful comments are present.
@SoDOOManiac
Copy link
Collaborator Author

I should have proposed it to Choco in the first place.

@SoDOOManiac SoDOOManiac deleted the MF_TRANSSHIFT-Doom-comment branch August 25, 2023 07:13
@SoDOOManiac
Copy link
Collaborator Author

@fabiangreffrath, I have no rights to submit pull requests to Choco.
Maybe you could submit a pull request? That Hmm?.. comment looks really strange.

@SoDOOManiac SoDOOManiac restored the MF_TRANSSHIFT-Doom-comment branch August 25, 2023 08:02
@SoDOOManiac
Copy link
Collaborator Author

if you think it's appropriate in Crispy but not Choco (e.g. some Choco philosophy regarding comments), feel free to merge this :)

@SoDOOManiac SoDOOManiac reopened this Aug 25, 2023
@SoDOOManiac SoDOOManiac merged commit 8b40be0 into master Aug 25, 2023
12 checks passed
@SoDOOManiac SoDOOManiac deleted the MF_TRANSSHIFT-Doom-comment branch August 25, 2023 08:50
@fabiangreffrath
Copy link
Owner

To be honest, I am not a fan of "fixing" Vanilla comments, either. If you think this needs clarification, an additional comment formulated as some kind of "answer" to the "Hm?" question would be appropriate.

@SoDOOManiac SoDOOManiac restored the MF_TRANSSHIFT-Doom-comment branch August 29, 2023 13:49
@SoDOOManiac
Copy link
Collaborator Author

OK, submitted another pull request preserving vanilla comment as a question and my comment as the answer.

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