libretro / retroarch-joypad-autoconfig

RetroArch joypad autoconfig files
MIT License
288 stars 364 forks source link

Renamed 347 .cfg file names: Replaced white space with underscore #1062

Closed davidhedlund closed 10 months ago

offalynne commented 10 months ago

@RobLoach seems the garbage submissions are still out of control. I will unsubscribe from notifications for my own sanity but kindly suggest action taken to prevent further mangling of the repo's commit log by this user's content free zero-value pull requests

davidhedlund commented 10 months ago

@RobLoach seems the garbage submissions are still out of control. I will unsubscribe from notifications for my own sanity but kindly suggest action taken to prevent further mangling of the repo's commit log by this user's content free zero-value pull requests

I apologize if I caused problems. I'm trying to standardize the filenames, and I haven't used git in years.

davidhedlund commented 10 months ago

content free zero-value pull requests

Aren't the file names changed in my pull request? Have I missed something?

RobLoach commented 10 months ago

@RobLoach seems the garbage submissions are still out of control. I will unsubscribe from notifications for my own sanity but kindly suggest action taken to prevent further mangling of the repo's commit log by this user's content free zero-value pull requests

No issue on un-watching for your sanity. The libretro team has many members to help keep things rolling. Mental health is important, and you are able to check in whenever you have the capability to do so. Thanks so much for helping out 👍

hizzlekizzle commented 10 months ago

Aren't the file names changed in my pull request? Have I missed something?

Yes, they're changed. No worries.

offalynne commented 10 months ago

I don't know if the conversation motivating this happened elsewhere but some opinions on why this should not be merged:

  1. Profile filenames match device names per fields both in the file contents and the HID descriptors. These changes break that

  2. In cases where a company rebrands by changing their spelling, this should not retroactively be applied to devices that still use the old branding on the physical device, packaging, descriptors, etc. as is the case of 8BitDo here

  3. The commit log is already a disaster as a result of untested garbage PRs from this user, further merges of erroneus piecemeal untested changes leaves significant work for others to deal with later

hizzlekizzle commented 10 months ago

I'm not aware of any other conversations about this either for or against. We usually try to be forgiving with people who are still learning how to contribute (though not to the extent that we want an avoidably messy commit log), but 1 and 2 seem like pretty big problems in any case.

davidhedlund commented 10 months ago

If other contributors have made some changes in the past such as the filename in the example you provided, that is not a good reason to change 300+ files on a mistaken assumption, especially when you have not tested the outcome of your changes.

davidhedlund commented 10 months ago

And it's not possible to submit a vice versa PR that replaces underscores with white spaces in all .cfg file names?

RobLoach commented 10 months ago

The goal is to have the input_device match the filename. The input_device is what the input driver discovers as the named ID of the controller. It also saves the device as [input_device].cfg, so we should keep them the same

davidhedlund commented 10 months ago

The goal is to have the input_device match the filename.

I've noticed that this is far away from standardized, since people are renaming either the filename or the input_device variable. A simple bash script for GNU/Linux can be used to automate this by setting the file name as input_device variable value for all .cfg files. What do you think about it? I can code it for you if you are interested.

The input_device is what the input driver discovers as the named ID of the controller. It also saves the device as [input_device].cfg, so we should keep them the same

offalynne commented 10 months ago

What do you think about it? I can code it for you if you are interested.

No.

The best choice is for the filename to be left alone so that it matches RetroArch's output to avoid contributors creating duplicates, however if past contributions have changed the file name these should be left alone as "cleaning up" is not a valid reason to create a pull request that is likely to raise further issues while solving none.

In general please do not create pull requests that touch mappings for devices you do not own as you will lack the capability to properly test your changes and confirm you have not in fact degraded the functionality of RetroArch Autoconfig.

davidhedlund commented 10 months ago

What do you think about it? I can code it for you if you are interested.

No.

The best choice is for the filename to be left alone so that it matches RetroArch's output to avoid contributors creating duplicates, however if past contributions have changed the file name these should be left alone as "cleaning up" is not a valid reason to create a pull request that is likely to raise further issues while solving none.

In general please do not create pull requests that touch mappings for devices you do not own as you will lack the capability to properly test your changes and confirm you have not in fact degraded the functionality RetroArch Autoconfig.

It was just an offer, thank you for being clear. Is this worth documentation @hizzlekizzle ?

davidhedlund commented 10 months ago

And it's not possible to submit a vice versa PR that replaces underscores with white spaces in all .cfg file names?

@offalynne What is your opinion?

hizzlekizzle commented 9 months ago

Is this worth documentation

Yes, I'd say the README could use some clarification on these topics, and "please don't touch files you can't personally test" seems like a good addition.