search

FuelLabs / fuels-ts

Fuel Network Typescript SDK
https://docs.fuel.network/docs/fuels-ts/
Apache License 2.0
44.39k stars 1.32k forks source link

Aggregate breaking change instructions in CHANGELOG #2609

Closed arboleya closed 1 day ago

arboleya commented 1 week ago

We currently use a digest to signal all PRs included new releases:

At the top, we have a section grouping all breaking changes with links to related PRs. Then, inside each PR, one can find detailed instructions about the breaking change it introduced.

The idea is to improve our CHANGELOG automation so that instructions from every PR are always included in the release notes, allowing users to find everything they need in a single place.

Template here.

arboleya commented 1 week ago

Blocked by:

calldelegation commented 1 week ago

@arboleya the GOAT 🙏 thank you

nedsalk commented 1 week ago

In relation to the implementation, we'd have to do the following two:

  1. Have a CI check to verify a breaking change PR contains a section in which the breaking change is explained
  2. On changelog generation, fetch the PR's description and copy that section over to the changelog

@arboleya the breaking change description on the PR would contain everything necessary to complete this issue, so how is this issue being blocked by #2523 which is concerned with issue creation?

arboleya commented 1 week ago

I will create a default/standard template for the PRs with a pre-defined breaking change section.

arboleya commented 5 days ago

@nedsalkI Please check the example I suggested here:

Can you please address that along with this issue?

Here's an example of how this PR could look with Migration Notes.

Breaking

Features

Fixes

Chores

Docs

Migration Notes

Features

The createConfig method now has individual hooks for all fuels CLI events types.

// Before
import { createConfig } from 'fuels';

createConfig({
  onSuccess: (event: CommandEvent, config: FuelsConfig) => {
    const { data, type } = event;
    console.log('fuels:onSuccess', { data, type });
  }
});

// After
import { createConfig, DeployedContract } from 'fuels';

createConfig({
  onBuild: (config: FuelsConfig) => {
    console.log('fuels:onBuild', { config });
  },
  onDeploy: (config: FuelsConfig, data: DeployedContract[]) => {
    console.log('fuels:onDeploy', { config, data });
  },
  onDev: (config: FuelsConfig) => {
    console.log('fuels:onDev', { config });
  },
  onNode: (config: FuelsConfig) => {
    console.log('fuels:onNode', { config });
  },
});

Altered the signature for onFailure hook in createConfig.

// Before
import { createConfig } from 'fuels';

createConfig({
  onFailure: (error: Error, config: FuelsConfig) => {
    console.log('fuels:onFailure', { error, config });
  },
});

// After
import { createConfig } from 'fuels';

createConfig({
  onFailure: (config: FuelsConfig, error: Error) => {
    console.log('fuels:onFailure', { config, error });
  },
});

Chores

The shorthand flag (-p) for passing through a path to various fuels CLI commands has been deprecated.

Migrate to using the --path flag instead.

# before
pnpm fuels -p ./my/path
# before
pnpm fuels --path ./my/path
SilentCicero commented 3 days ago

@arboleya if you can also include a small section which describes the changes from a high level, i.e. a description of the update: In this update we did X, Y, Z to improve A, B and C.

A breif description we can pull in for the update in plain english will be helpful. It can be short, but just describe what the team was up to when they updated for the version.

The other Chores and Migration Notes section I see listed in the lower comments here seems perfect for what we need.

arboleya commented 3 days ago

@SilentCicero Do you mean a high-level description summarizing what is relevant about the PR?

Does it need to be fluid text, or can it still be bullet points or something similar?

These Release Notes are an automated amalgamation of individual PRs, so this info must still come from the PRs. There could be an optional section that people can fill out on each PR, provided their PR is relevant enough. Then, we can pull and combine those at the top before the digest begins — would that work?

If a given release doesn't have relevant notes, we could have a default one: