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

Action to mass rename all ROMs for a platform #253

Closed
wants to merge 5 commits into from
Closed

Action to mass rename all ROMs for a platform #253

wants to merge 5 commits into from

Conversation

gantoine
Copy link
Member

@gantoine gantoine commented May 17, 2023

Closes #251

This PR introduces a new socket endpoint that allows mass-renaming all ROMs for a platform (when in the Gallery view). This should make it easier for users with large libraries who's games have been correctly categorized to clean up their files.

What changes

  • New socket endpoint mass_rename
    • Calls new rename_all_roms with a given platform slug
  • Socket event names are now prepended with the endpoint
    • scan:done, scan:done_ko, mass_rename:done
  • Get eslint working with a basic config and ignore file

At some point I'd like to add a simple prettier config (and settle on consistent code style).

TODO

@gantoine gantoine self-assigned this May 18, 2023
@zurdi15 zurdi15 self-requested a review May 18, 2023 07:55
@zurdi15
Copy link
Member

zurdi15 commented May 18, 2023

Looks really nice, didn't know about the good practice of prepend sockets events with the endpoint.

Apart from that, I think it is better to open the pull requests to develop and when it has enough features/bug fixes and have been properly tested to release a new version, a PR against master can be opened.

Also I have the bad practice of sometimes, for small fixes or refactor changes, working directly on develop when I am the only developer, so I pushed the last commits to develop I had in local and I will start to work strictly with branches and making sure to rebase/merge develop before opening a PR, so in this case you should change the PR against develop.

I also changed the base branch for the repo to avoid open PRs to master

backend/endpoints/rom.py Outdated Show resolved Hide resolved
@gantoine
Copy link
Member Author

Apart from that, I think it is better to open the pull requests to develop and when it has enough features/bug fixes and have been properly tested to release a new version, a PR against master can be opened.

That's debatable: in most open-source projects, master/main is the bleeding edge, and work should be branched from it. If you'd like to keep releases in a separate branch, you could have a release branch (or release-x.x.x branches) and merge commits into it when they're tested and ready to deploy.

For simplicity's sake, I'd recommend switching the base branch back to master.

I pushed the last commits to develop I had in local and I will start to work strictly with branches

That's wise, I do the same thing for the projects I maintain.

@zurdi15
Copy link
Member

zurdi15 commented May 18, 2023

Apart from that, I think it is better to open the pull requests to develop and when it has enough features/bug fixes and have been properly tested to release a new version, a PR against master can be opened.

That's debatable: in most open-source projects, master/main is the bleeding edge, and work should be branched from it. If you'd like to keep releases in a separate branch, you could have a release branch (or release-x.x.x branches) and merge commits into it when they're tested and ready to deploy.

For simplicity's sake, I'd recommend switching the base branch back to master.

I pushed the last commits to develop I had in local and I will start to work strictly with branches

That's wise, I do the same thing for the projects I maintain.

You convinced me. Anyway is mostly like the same method I am proposing but instead of master/develop it will be release/master. At the end I want a branch to merge all the PR's and test it with all the PR's integrated but having an extra branch for the releases. Already reverted the base branch change.

@zurdi15
Copy link
Member

zurdi15 commented May 18, 2023

Since you are making a new socket endpoint and refactoring some parts of the front (and back) code, I will merge this PR with the API wrapper to let you adapt the code and use it if needed. Maybe we can add the socket logic into the wrapper

@gantoine
Copy link
Member Author

@zurdi15 After thinking about this today, I think there's some preliminary work that needs to happen before we can tackle a feature like this (namely an integration with no-intro). I'm going to close this PR in a bit, after I've extracted some of the refactoring we can still use into a separate PR.

@gantoine gantoine closed this May 18, 2023
@zurdi15
Copy link
Member

zurdi15 commented May 18, 2023

Yeah would be nice to first integrate no-intro in order to have a proper renaming feature 👍

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.

[Feature] Library Section: Add Option to mass rename properly identified files
2 participants