gothic-1-community-patch icon indicating copy to clipboard operation
gothic-1-community-patch copied to clipboard

Automate new bugfix

Open AmProsius opened this issue 3 years ago • 5 comments

To speed up bug fixing, the process to create the new fix can (potentially) be covered with a script. These are the steps to create the files and file changes for a new fix:

  • [x] Create a new branch: git checkout -b bugXXX.
  • [x] Add English issue fix with link to ./CHANGELOG.md.
  • [x] Add German issue fix with link to ./docs/changelog_de.md.
  • [x] Add fix file name to ./src/Ninja/G1CP/Content_G1.src.
  • [x] Add fix function name to ./src/Ninja/G1CP/Content/patchInit.d or ./src/Ninja/G1CP/Content/Fixes/gamesave.d respectively.
  • [x] Add fix file to ./src/Ninja/G1CP/Content/Fixes/Session/ or ./src/Ninja/G1CP/Content/Fixes/Gamesave/ respectively.
  • [x] Add test file name to ./src/Ninja/G1CP/Testsuite.src.
  • [x] Add test file to ./src/Ninja/G1CP/Content/Tests/.
  • [x] Optionally commit the automation: git commit -m "Automatically add files for fix #XXX".
  • [x] Optionally turn the issue into a pull request: hub pull-request -i XXX.

Variables to know:

  • Issue number (XXX)
  • Issue name (optional)
  • Fix file/function name (FixFunctionName)
  • Fix type (session or gamesave)
  • Test type (auto or manual)
  • English changelog entry
  • German changelog entry

Things to consider:

  • Make sure the function file name does not exceed 45 characters (G1CP_XXX_NotMoreThan45Characters).
  • The fix file can be filled with a template. Something like this:
/*
 * #XXX Issue name
 */
func int G1CP_XXX_FixFunctionName() {
    /* todo: Write the fix */
    return FALSE;
};
  • 💡 Maybe we can even define fix types to further develop the fix file template.
  • The test file can be filled with a template according to the fix type. Something like this:
/* Manual */

/*
 * #XXX Issue name
 *
 * There does not seem an easy way to test this fix programmatically, so this test relies on manual confirmation.
 *
 * Expected behavior: todo.
 */
func void G1CP_Test_XXX() {
    G1CP_Testsuite_CheckManual();
    /* todo: Write checks */
    G1CP_Testsuite_CheckPassed();

    /* todo: Write the test */
};
/* Automatic */

/*
 * #XXX Issue name
 */
func int G1CP_Test_XXX() {
    /* todo: Write checks */
    G1CP_Testsuite_CheckPassed();

    /* todo: Write the test and adjust the return value */
    return FALSE;
};

AmProsius avatar Jan 11 '22 09:01 AmProsius

I added another step:

  1. Optionally turn the issue into a pull request: hub pull-request -i XXX.

szapp avatar Jan 12 '22 16:01 szapp

I wonder if this should be a local script, that a collaborator triggers on their own machine, or a Github bot that commits the automated changes and turns the issue into a PR once triggered.

The latter would have the advantage that all information is available to the bot, and it can automatically generate all the files.

szapp avatar Jan 13 '22 10:01 szapp

Great idea! The bot could also turn the PR into a draft after converting the issue.

AmProsius avatar Jan 13 '22 10:01 AmProsius

It would be awesome to somehow encode the necessary info in the issue.

Variables to know:

  • Issue number (XXX)
  • Issue name (optional)
  • Fix file/function name (FixFunctionName)
  • Fix type (session or gamesave)
  • English changelog entry
  • German changelog entry

Out of those listed above, we have all of them clearly extractable except for

  • Fix file/function name (FixFunctionName)
  • English changelog entry
  • German changelog entry

I am sure the bot could parse the issue description and comments for any key words. We'd have to add them manually.

szapp avatar Jan 13 '22 10:01 szapp

I am sure the bot could parse the issue description and comments for any key words. We'd have to add them manually.

I have no problem with adding both changelog entries to the issue's description (I already write the Expected behavior like the English changelog entry). The fix function name is unfortunate, because it doesn't really belong to the issue's description, but its okay for me to add it for the automation.

AmProsius avatar Jan 13 '22 10:01 AmProsius