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

Changed all instances of the role "infantry_support" to "inf_support" #5644

Closed
wants to merge 1 commit into from

Conversation

repligator
Copy link
Collaborator

Changed all instances of the role "infantry_support" to "inf_support". This doesn't actually change the function of anything, just makes troubleshooting easier. See PR #5289 and Issue #5267.

Ran 317 tests, 312 passed, 5 ignored. Also manually tested Force Generator to make sure the effected units and formations still appear.

…. This doesn't actually change the function of anything, just makes troubleshooting easier. See PR MegaMek#5289 and Issue MegaMek#5267.

Ran 317 tests, 312 passed, 5 ignored. Also manually tested Force Generator to make sure the effected units and formations still appear.
@HammerGS
Copy link
Member

Just a question on this if the issue is between code and notes, why don't we fix the notes as Infantry_Support in the code seems clearer?

@repligator
Copy link
Collaborator Author

Either is fine with me.

@IllianiCBT
Copy link
Collaborator

Do we have a precedent for uses of inf_x vs infantry_x in the roles? If so, we probably still want to switch to inf_x for consistency. If not, then just changing the readme is probably the cleanest approach.

@HammerGS
Copy link
Member

Can we change the supporting documentation rather than code. Going forward we can ensure X_support the X can be a full name to be very clear what we are looking at,.

HammerGS added a commit that referenced this pull request Jun 30, 2024
@HammerGS
Copy link
Member

Closing as addressed with 0274235

@HammerGS HammerGS closed this Jun 30, 2024
@repligator repligator deleted the Role-name-fun branch August 9, 2024 18:50
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.

3 participants