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

Demo the use of CHANGELOG.md for managing library and tool versions #2095

Merged
merged 11 commits into from
Feb 18, 2022
Merged
1,102 changes: 1,102 additions & 0 deletions CHANGELOG.md

Large diffs are not rendered by default.

69 changes: 58 additions & 11 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -149,25 +149,41 @@ That way, we can ensure the issue stays fixed after closing it.

## Guidelines

- Please always rebase your code on the targeted branch.
To keep your fork up to date, run this command:
> git remote add upstream https://github.com/fsprojects/fantomas.git
### Target branch

Updating your fork:
Please always rebase your code on the targeted branch.
To keep your fork up to date, run this command:
> git remote add upstream https://github.com/fsprojects/fantomas.git

> git checkout master && git fetch upstream && git rebase upstream/master && git push
Updating your fork:

> git checkout master && git fetch upstream && git rebase upstream/master && git push

### Unit test

- Unit test names should start with a lowercase letter.
- Verify if the change you are making should also apply to signature files (`*.fsi`).
- Check if you need additional tests to cope with a different combination of settings.
- Check if you need additional tests to cope with a different combination of defines (`#if DEBUG`, ...).
- Write/update documentation when necessary.
- When creating a test that is linked to a GitHub issue, add the number at the back with a comma, as in the following:

```fsharp
[<Test>]
let ``preserve compile directive between piped functions, 512`` () = ...
```

### Verify signature files

Verify if the change you are making should also apply to signature files (`*.fsi`).

### Verify slight variations

- Check if you need additional tests to cope with a different combination of settings.
- Check if you need additional tests to cope with a different combination of defines (`#if DEBUG`, ...).

### Documentation

Write/update documentation when necessary.

### Pull request title

- Give your PR a meaningful title. Make sure it covers the change you are introducing in Fantomas.

For example:
Expand All @@ -178,10 +194,41 @@ let ``preserve compile directive between piped functions, 512`` () = ...
- Not mandatory, but when fixing a bug consider using `fix-<issue-number>` as the git branch name.<br />
For example, `git checkout -b fix-1404`.

### Format your changes

- Code should be formatted to our standard style, using either `dotnet fake run build.fsx -t Format` which works on all files, or
`dotnet fake run build.fsx -t FormatChanged` to just change the files in git.
- If you forget, there's a git `pre-push` script that will run this for you, make sure to run `dotnet fake build -t EnsureRepoConfig` to set that hook up.

### Changelog

- Add an entry to the `CHANGELOG.md` in the `Unreleased` section based on what kind of change your change is. Follow the guidelines at [KeepAChangelog](https://keepachangelog.com/en/1.0.0/#how) to make your message relevant to future readers.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any way to enforce this via our main build pipeline?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not easily, I suppose you could do some custom check like 'the commits in this PR must have changed CHANGELOG.md', but that's a bit finicky to get correct. I might suggest making a pull request template that reminds users/provides a checklist for them to work though (this template could point to sections of the CONTRIBUTING.md for clarification, for example).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, of course not everything needs to be in there as well.
For example, today I discovered there is an unused function in TokenParser. At some point, I'll submit a PR to remove it.
But such a thing should not really make the release notes.

Let me think about the proper guidance of this. If a bug was solved, we definitely need an entry there.

Sorry, this is taking me some time to merge this one in. I'd just want to go over this one slowly to make sure I grasp everything.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No worries at all - I want you to be completely comfortable with the process at the end of this, and the tool is very young, so I completely understand wanting to have a firm grasp on the mechanics.

If you prefer, I can remove the guidance about changelogs from the CONTRIBUTING doc, and you can keep the writing of changelogs as a release-time activity, as you do now. In that case nothing about your workflow would change at all.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I need to give it some more thought but I like the idea that these things are captured sooner in the development cycle.
There is nothing complicated in mentioning you fixed a bug in the changelog.
I need to think about a set of practices when to add what message.
There really are upfront moments when you know it should be included in the release notes.
I welcome the idea that this becomes a joint effort.

- If you're not sure what Changelog section your change belongs to, start with `Changed` and ask for clarification in your Pull Request
- If there's not an `Unreleased` section in the `CHANGELOG.md`, create one at the top above the most recent version like so:

```markdown
## [Unreleased]

### Changed
* Your new feature goes here

## [4.7.4] - 2022-02-10

### Added
* Awesome feature number one
```

- When fixing a `bug (soundness)`, add a line in the following format to `Fixed`:
`* <Original GitHub issue title> [#issue-number](https://github.com/fsprojects/fantomas/issues/issue-number)`.
For example, `* Spaces are lost in multi range expression. [#2071](https://github.com/fsprojects/fantomas/issues/2071)`.
Do the same, if you fixed a `bug (stylistic)` that is not related to any style guide.
- When fixing a `bug (stylistic)`, add a line in the following format to `Changed`
`Update style of xyz. [#issue-number](https://github.com/fsprojects/fantomas/issues/issue-number)`
- For example, `* Update style of lambda argument. [#1871](https://github.com/fsprojects/fantomas/issues/1871)`.

### Run a local build

- Finally, make sure to run `dotnet fake build`. Among other things, this will check the format of the code and will tell you, if
Finally, make sure to run `dotnet fake build`. Among other things, this will check the format of the code and will tell you, if
your changes caused any tests to fail.

### Small steps
Expand Down Expand Up @@ -267,7 +314,7 @@ When faced with this issue, ask yourself the following questions:
- Is the trivia being printed in `CodePrinter.fs` when the trivia node is processed?

Without any debugging, these questions can already point you in the right direction to solve the problem.
You might be surprised how little code change was necessary to resolve the bug ;) See, for example, #1130, which solved a problem like this in only five lines.
You might be surprised how little code change was necessary to resolve the bug ;) See, for example, [#1130](https://github.com/fsprojects/fantomas/pull/1130), which solved a problem like this in only five lines.

### Repeating newline bug

Expand Down
31 changes: 31 additions & 0 deletions Directory.Build.props
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<Project>
<PropertyGroup>
<!-- Set up version and package release note generation from this changelog. -->
<ChangelogFile>$(MSBuildThisFileDirectory)CHANGELOG.md</ChangelogFile>
<!-- Common packaging properties for all packages in this repo -->
<Authors>Florian Verdonck, Jindřich Ivánek</Authors>
<PackageRequireLicenseAcceptance>false</PackageRequireLicenseAcceptance>
<GenerateDocumentationFile>true</GenerateDocumentationFile>
<Description>
This library aims at formatting F# source files based on a given configuration.
Fantomas will ensure correct indentation and consistent spacing between elements in the source files.
Some common use cases include:
(1) Reformatting a code base to conform a universal page width
(2) Converting legacy code from verbose syntax to light syntax
(3) Formatting auto-generated F# signatures.
</Description>
<Copyright>Copyright © $([System.DateTime]::UtcNow.Year)</Copyright>
<PackageTags>F# fsharp formatting beautifier indentation indenter</PackageTags>
<EmbedUntrackedSources>true</EmbedUntrackedSources>
<PublishRepositoryUrl>true</PublishRepositoryUrl>
<DebugType>embedded</DebugType>
<PackageIcon>fantomas_logo.png</PackageIcon>
<PackageLicenseExpression>Apache-2.0</PackageLicenseExpression>
<PackageReadmeFile>README.md</PackageReadmeFile>
</PropertyGroup>

<ItemGroup Condition="'$(IsPackable)' == 'true'">
<None Include="$(MSBuildThisFileDirectory)fantomas_logo.png" Visible="false" Pack="true" PackagePath="" />
<None Include="$(MSBuildThisFileDirectory)README.md" Visible="false" Pack="true" PackagePath="" />
</ItemGroup>
</Project>
Loading