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

Feedback/call for input regarding release frequency/guidelines #370

Open
xiashtra opened this issue Aug 24, 2024 · 3 comments
Open

Feedback/call for input regarding release frequency/guidelines #370

xiashtra opened this issue Aug 24, 2024 · 3 comments
Labels
documentation Improvements or additions to documentation enhancement New feature or change to existing functionality needs-review Awaiting review

Comments

@xiashtra
Copy link
Collaborator

xiashtra commented Aug 24, 2024

Feature(s) Requested

Per discussion with @wexxlee regarding version release bumps, I think it may be beneficial to set some guidelines on release frequency; when to expedite a release; who should feel free to PR a release bump (anyone!); and overall general suggestions on how to handle release timing.

  • For starters, I believe releases should be expedited when there are PRs merged which resolve an issue that users are facing. For example, not having a release after i18n: add r4s translations #334 led to several reports from non-EN users of non-working triggers in M4S, similar to No calls/alerts in M4S  #368. I advocated for merging build: Bump version to 0.32.7 #369 asap to resolve these issues, rather than waiting on other pending PRs like raidboss: Initial Deadwalk package #360 to be merged first. Similar situations may call for some guidelines regarding classifying PR "importance" when deciding if there should be an immediate release bump to go along with a merge.
  • Regarding release frequency after a new content patch, I believe that we should err on the side of more frequent releases, to get new coverage into the hands of users faster. This also lets i18n translators start working quicker, and gives us better opportunities to gather user feedback/error reports sooner. For a suggested release frequency, I think even as often as once per day would be acceptable following a new raid tier. We have had nearly that level of frequency during past Ultimate releases.
  • Going hand-in-hand with a higher release frequency during new content drops, I'd also suggest a preference for more, smaller PRs rather than waiting for one large PR that covers every mechanic of a new instance. Smaller PRs are easier to review and iterate on, and I think they offer better opportunities for collaboration.
  • For periods outside of a new content patch, I think a release cadence of once every two weeks when there are unreleased merges pending wouldn't be too frequent.
  • I'd also like to remind everyone that anyone can PR a release bump, and should feel free to do so. Especially our i18n contributors; when there are large batches of new translations being merged that is a valid reason for a bump.

Feel free to comment on the above or add anything else you think is missing.

@xiashtra xiashtra added enhancement New feature or change to existing functionality needs-review Awaiting review documentation Improvements or additions to documentation labels Aug 24, 2024
@wexxlee
Copy link
Collaborator

wexxlee commented Aug 24, 2024

Well said, and thanks for kicking off the thread! Agree 100%. The only additional thoughts I'd add:

  • Want to strongly reiterate that any contributor can and should PR a version bump whenever they feel it's warranted. I think version bumps are low-burden activities for users, especially given the way the auto-updater works and that ACT's use of deucalion means an update/restart of ACT no longer requires a game restart. The process for doing so can be found here, and it's very straightforward and largely automated.
  • If folks are generally aligned on the guidelines above (or if it gets tweaked when others weigh in), I think it'd be a good idea to update CONTRIBUTING.md with these guidelines.

@valarnin
Copy link
Collaborator

* For starters, I believe releases should be expedited when there are PRs merged which resolve an issue that users are facing.  For example, not having a release after [i18n: add r4s translations #334](https://github.com/OverlayPlugin/cactbot/pull/334) led to several reports from non-EN users of non-working triggers in M4S, similar to [No calls/alerts in M4S  #368](https://github.com/OverlayPlugin/cactbot/issues/368).  I advocated for merging [build: Bump version to 0.32.7 #369](https://github.com/OverlayPlugin/cactbot/pull/369) asap to resolve these issues, rather than waiting on other pending PRs like [raidboss: Initial Deadwalk package #360](https://github.com/OverlayPlugin/cactbot/pull/360) to be merged first.  Similar situations may call for some guidelines regarding classifying PR "importance" when deciding if there should be an immediate release bump to go along with a merge.

Strongly agree. Any user-facing bugfix that impacts recent content (maybe current + previous expansion?) should be grounds for an immediate version bump PR, unless there is some critical reason to hold off (e.g. some other change has been made which is not ready for release yet).

* Regarding release frequency after a new content patch, I believe that we should err on the side of more frequent releases, to get new coverage into the hands of users faster.  This also lets i8n translators start working quicker, and gives us better opportunities to gather user feedback/error reports sooner.  For a suggested release frequency, I think even as often as once per day would be acceptable following a new raid tier.  We have had nearly that level of frequency during past Ultimate releases.
* Going hand-in-hand with a higher release frequency during new content drops, I'd also suggest a preference for more, smaller PRs rather than waiting for one large PR that covers every mechanic of a new instance.  Smaller PRs are easier to review and iterate on, and I think they offer better opportunities for collaboration.

I think it makes sense to get a timeline in as a separate PR, since they're generally easier to review and merge. For triggers, it's a bit more complicated because multiple people working on triggers for the same fight is almost always going to result in merge conflicts.

I think that making it feasible for multiple people to work on triggers for the same fight at the same time will require an initial PR to put in guard rails of some sort, so that git has hard anchor points in the code to reference to avoid merge conflicts.

It may make sense to do an initial PR after the fight has been explored a bit that breaks down the fight into segments and then adds comment headers for those segments to the file as an initial PR, e.g. for Wicked Thunder savage, an initial PR before any actual triggers were written could look like:

// ...
export interface Data extends RaidbossData {
  phase: Phase;
  /*
   * Phase 1
   */
  /*
   * Bewitching Flight
   */
  /*
   * Witch Hunt
   */
  /*
   * EE1
   */
  /*
   * EE2
   */
  /*
   * Ion Cannon
   */
  /*
   * Transition
   */
  /*
   * Phase 2
   */
  /*
   * Mustard Bomb
   */
  /*
   * etc etc repeat for each mini-phase
   */
// ...
  triggers: [
    // ... Repeat header segments from `Data` here

This would serve a secondary purpose of better organizing the triggers and data for the fight.

* For periods outside of a new content patch, I think a release cadence of once every two weeks when there are unreleased merges pending wouldn't be too frequent.

Release cadence outside of new content has always felt like a judgement call to me, e.g. only release if there's enough stuff to actually release something for. I do hear complaints from friends about the frequency of updates sometimes.

I've always looked at it as "Will the changes since the last release materially change the user experience in some manner? If so, it's worth doing a release for."

* I'd also like to remind everyone that anyone can PR a release bump, and should feel free to do so.  Especially our i8n contributors; when there are large batches of new translations being merged that is a valid reason for a bump.

Yes. Please, PR a version bump for translations. Our translation contributors do a ton of great work and it's hard to judge when something is translated enough to be ready for a PR, as a non-translator.


* Want to strongly reiterate that any contributor can and should PR a version bump whenever they feel it's warranted.  I think version bumps are low-burden activities for users, especially given the way the auto-updater works and that ACT's use of deucalion means an update/restart of ACT no longer requires a game restart.  The process for doing so can be found [here](https://github.com/OverlayPlugin/cactbot/blob/v0.32.7/CONTRIBUTING.md#how-to-release), and it's very straightforward and largely automated.

The complaints I always hear from users about updates are that they take too long to apply and restart ACT for. Because most of those users are people that show up to raid 2 minutes before start time and haven't turned on ACT or FFXIV since the previous raid night.

* If folks are generally aligned on the guidelines above (or if it gets tweaked when others weigh in), I think it'd be a good idea to update `CONTRIBUTING.md` with these guidelines.

I think that it may be worth a pass through of all of our documentation at some point after the tier has died down a bit and people have more free time, to update various things. For example, there's a requirements.txt file that lists python requirements despite us no longer having python code.

So it may be worth it to throw up a general "documentation updates" issue as well, and possibly tack any CONTRIBUTING.md updates on to that issue.

@xiashtra
Copy link
Collaborator Author

Another thought: we could probably bump any time there is a patch requiring a FFXIV_ACT_Plugin or OverlayPlugin update, even if it's only for a few small changes. Since users will be disrupted by those updates already, it would make sense to piggy-back our updates to happen at the same time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or change to existing functionality needs-review Awaiting review
Projects
None yet
Development

No branches or pull requests

3 participants