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

Development: use standardised PR title prefixes? #1577

Closed
agoose77 opened this issue Aug 3, 2022 · 7 comments
Closed

Development: use standardised PR title prefixes? #1577

agoose77 opened this issue Aug 3, 2022 · 7 comments
Assignees

Comments

@agoose77
Copy link
Collaborator

agoose77 commented Aug 3, 2022

Background

I'd like to introduce a tool like https://github.com/executablebooks/github-activity to automate our release note generation. We already have a tool that does this, but we don't do anything special with respect to categorizing the PRs that are merged during the release cycle.

The benefit of using another tool like github-activity is twofold:

  • other people are using & developing it, so we won't have to maintain our own implementation
  • richer release notes - e.g. easier to find bugfixes vs features

For example, here's a generated changelog entry for 1.8.0:

New changelog for 1.8.0

1.8.0

(full changelog)

New features added

Bugs fixed

Maintenance and upkeep improvements

Documentation improvements

API and Breaking Changes

Other merged PRs

Contributors to this release

(GitHub contributors page for this release)

@agoose77 | @allcontributors | @BioGeek | @chrisburr | @codecov | @dependabot | @douglasdavis | @henryiii | @ianna | @ioanaif | @jpivarski | @martindurant | @miranov25 | @pre-commit-ci | @swishdiff

Existing changelog for 1.8.0

Release 1.8.0

(pip)

(no pull requests)

Release 1.8.0rc7

(pip)

  • PR #1326: Docs: fix typo in documentation.

  • PR #1314: chore: remove extra files from the wheels.

  • PR #1313: ci: avoid PyPI cuda wheel upload.

  • PR #1316: chore: bump pybind11 to 2.9.1.

  • PR #1312: Keep as much length knowledge as possible in typetracers.

  • PR #1322: chore: wheel not required for setuptools PEP 517 (all-repos)

  • PR #1317: C++ refactoring: ak.cartesian, ak.argcartesian.

  • PR #1301: C++ refactoring: ak.strings_astype.

  • PR #1308: Feat: add optiontype_outside_record argument to ak.zip

  • PR #1307: C++ refactoring: ak.argcombinations, ak.combinations.

  • PR #1309: C++ refactoring: ak.sort.

Release 1.8.0rc6

(pip)

  • PR #1310: Fix lost ‘behavior’ in ‘ak.unzip’.

  • PR #1300: Implement Awkward –> C++ with Cling.

  • PR #1306: Version of awkward_cuda should be tied with awkward.

  • PR #1304: C++ refactoring: ak.argsort.

  • PR #1303: Fix: do not increment field index for nested lists.

Release 1.8.0rc5

(pip)

(no pull requests)

Release 1.8.0rc4

(pip)

  • PR #1299: Remove unnecessary line blank from the tops of almost all files .

  • PR #1298: Allow ak.nan_to_num arguments to be arrays.

  • PR #1296: ak.fields.

  • PR #1297: ak.without_parameters.

  • PR #1293: C++ refactoring: ak.full_like, ak.zeros_like, ak.ones_like.

  • PR #1275: style: pylint 1.

  • PR #1274: Fixing #1266 (in v1 and v2), possibly by reordering nextparents.

  • PR #1294: C++ refactoring: ak._v2.from_arrow_schema function.

  • PR #1289: C++ refactoring: ak.with_parameter.

  • PR #1292: C++ refactoring: ak.with_field.

  • PR #1290: typo.

  • PR #1276: This PR adds support to call kernels in CUDA from v2 Awkward Arrays.

  • PR #1249: Fix: support nested option types in ak.is_none (also: #1193, #1193)

  • PR #1279: Fix: simplify output in {Byte,Bit}MaskedArray.

  • PR #1277: [pre-commit.ci] pre-commit autoupdate.

  • PR #1240: Getting Numba to work for v2 arrays.

  • PR #1207: json de-/serialisation from/to string or file.

  • PR #1270: Add GHA to build CUDA Wheels and update the cuda build script.

  • PR #1262: chore: initial nox and pylint support.

  • PR #1267: style: update to first non-pre-release black!

  • PR #1265: Bump pypa/gh-action-pypi-publish from 1.4.2 to 1.5.0.

  • PR #1257: Add a .zenodo.json file to specify a set of authors.

  • PR #1264: Bump pypa/cibuildwheel from 1.12.0 to 2.3.1.

  • PR #1263: chore: add dependabot for actions.

  • PR #1259: Fix: fix ByteMaskedArray.simplify_optiontype()

  • PR #1258: Remove distutils reference in test (now an error).

  • PR #1255: chore: update pytest config, 6.0+

  • PR #1242: C++ refactoring: ak.parameters.

  • PR #1243: style: add shellcheck.

  • PR #1254: fix: building twice was broken.

  • PR #1248: Fix: support mixed array types in NumpyLike.to_rectilinear

  • PR #1246: style: further cleanup for Python 3.6+

  • PR #1244: style: pyupgrade to 3.6.

  • PR #1245: layout.completely_flatten should not concatenate (performance issue).

  • PR #1234: C++ refactoring: ak.type and ak.values_astype.

  • PR #1214: Fix: drop parameters for flattened RecordArray.

Release 1.8.0rc3

(pip)

  • PR #1239: Revert “Build wheels for ppc64le (#1224)”

Release 1.8.0rc2

  • PR #1233: C++ refactoring: ak.with_name.

  • PR #1231: Updated the generate-cuda script. Works for py >= 3.8.

  • PR #1224: Build wheels for ppc64le.

  • PR #1237: Remove Windows 32-bit from the Python 3.10 build.

  • PR #1229: C++ refactoring: ak.pad_none.

  • PR #1232: macos segfault bugfix.

  • PR #1225: C++ refactoring: ak.zip.

  • PR #1228: Redo PR #1227: fixing ‘emptyArray’ typo.

  • PR #1226: C++ refactoring: ak.num.

  • PR #1217: C++ refactoring: ak.flatten.

  • PR #1220: C++ refactoring: ak.where.

  • PR #1223: Restore pybind11 2.9.0.

  • PR #1218: Make highlevel __repr__ safe for typetracers.

  • PR #1219: C++ refactoring: ak.mask.

  • PR #1221: C++ refactoring: ak.local_index.

  • PR #1222: C++ refactoring: ak.ravel.

  • PR #1211: Removed v1_to_v2 from all v2 tests. (also: #962)

  • PR #1215: Fixed handling of list-nested boolean slices.

  • PR #1212: Drop Win32 Py3.10 test and musllinux in deployment.

  • PR #1213: [pre-commit.ci] pre-commit autoupdate.

Release 1.8.0rc1

  • PR #1188: ci: try Numba RC on 3.10.

  • PR #1199: chore: bump to pybind11 2.9.0.

  • PR #1210: docs: add BioGeek as a contributor for doc.

  • PR #1208: ak._v2 namespace is now filled with the right symbols.

  • PR #1206: Highlevel non-reducers and improved testing/fixes for reducers.

  • PR #1204: ak._v2.operations.convert.to_numpy is done.

  • PR #1203: Don’t let ak.to_list act on v2 arrays (finishing #1201).

  • PR #1202: Better error message for Content::axis_wrap_if_negative.

  • PR #1201: Implemented v2 ak.to_list and switched all v2 tests to use it.

  • PR #1198: Allow non-array iterables in __array_function__.

  • PR #1197: Fix ak.singletons for non-optional data.

  • PR #1196: Remove distutils dependence.

  • PR #1195: Fix: _pack_layout should also pack projected index arrays.

  • PR #1194: [pre-commit.ci] pre-commit autoupdate.

  • PR #948: pictures for a tutorial.

  • PR #1155: ArrayBuilder: replace shared with unique.

  • PR #1011: chore: bump pybind11 to 2.8.0.

  • PR #1186: feat: bump cibuildwheel, add Python 3.10.

  • PR #1187: Remove duplicated text.

  • PR #1184: Drop all length information from TypeTracer, get all tests working again.

  • PR #1183: Bugs found by the Dask project: broaden type-tracers’ applicability.

  • PR #1172: First bug found by @martindurant.

  • PR #1182: Remove Python 2.7 and 3.5 support.

  • PR #1181: Fixed zeros in RegularArray shape.

  • PR #1175: NumpyArray::numbers_to_type must use flattened_length, not length.

  • PR #1180: ak.to_numpy with RegularArray of size zero and non-zero length.

  • PR #1179: Raise ValueError for incompatible union types in ak.unzip.

  • PR #1178: Fix leading zeros in ak.unflatten.

  • PR #1174: [pre-commit.ci] pre-commit autoupdate.

Discussion

In order to produce richer release notes, we would benefit from using a standard set of prefixes for our PR titles. Really, we could do this for commits too, but as we squash and merge, it's really an aside for the purposes of this discussion. Some examples of these standards:

It really doesn't matter what we choose here, so long as we are all following some kind of convention. We don't even need to use the same convention (although this is really recommended).

I could write a small action to validate the PR title before merge if that would be helpful to enforce this. I hope that, given we only need to do this at merge time, it should minimally impact us during development.

Thoughts?

@jpivarski
Copy link
Member

I'm in favor of this. The main thing, though, is that we want to cut over from our current state of not using it to 100% using it. If there's something that would enforce it, to make sure that we don't overlook it or make a mistake, that would be helpful.

At the same time, the tags and release names are not standard (e.g. "v2.0.0" and "Version 2.0.0") and that's because we were following an in-house standard until now, and should switch over all at once, maybe at some auspicious time like version 2.0.0.

One thing I want to make sure of, though, is that the flake8 version bumps are either not in the release notes or are somehow segregated from the changes that are meaningful to developers and users.

@agoose77
Copy link
Collaborator Author

agoose77 commented Aug 3, 2022

I'm in favor of this. The main thing, though, is that we want to cut over from our current state of not using it to 100% using it. If there's something that would enforce it, to make sure that we don't overlook it or make a mistake, that would be helpful.

Agreed. I suggested that we use a CI action to catch this before merge?

At the same time, the tags and release names are not standard (e.g. "v2.0.0" and "Version 2.0.0") and that's because we were following an in-house standard until now, and should switch over all at once, maybe at some auspicious time like version 2.0.0.

Agreed

One thing I want to make sure of, though, is that the flake8 version bumps are either not in the release notes or are somehow segregated from the changes that are meaningful to developers and users.

The easiest solution would be to let these fall into the maintenance category. However, I'm discussing extending the categorisation support over at github-activity, so we could add another category for CI stuff.

@agoose77 agoose77 self-assigned this Aug 6, 2022
@agoose77
Copy link
Collaborator Author

@jpivarski can you ping whomever you consider a stakeholder to weigh in on this? Namely, which convention to choose. Here are a couple:

If people don't have any strong feelings (and I suspect that's the case), then we can just choose one. I prefer the Angular convention, though I've been title-casing the type which is probably the wrong way to do it.

@jpivarski
Copy link
Member

This would be a great thing to decide at our Awkward-Uproot group meeting; that's a gathering of the stakeholders. Could you prepare a slide/screenshot of what typical examples look like?—it could even be something you paste into this chat and screenshare during the meeting—and we'll pick one.

I don't think we'll have strong opinions, and I don't think it will take more than 5 minutes, but it gives us all a chance to see what we're committing to.

@henryiii
Copy link
Member

What's the difference between conventionalcommits and the angular variant? I always follow conventionalcommits (out of habit) unless a project has a different convention (like pypa/build). But it looks just like the angular variant.

@agoose77
Copy link
Collaborator Author

agoose77 commented Aug 18, 2022

Conventional Commits only specifies fix: and geat: (in addition to BREAKING CHANGE:). The Angular variant defines additional labels that are used by Angular JS, e.g. build:, chore:, etc. Additionally, Angular also specify their type field (fix:) to be lowercase.

There is no harm that I can see in choosing our own here, it's just nice to have inspiration.

@agoose77
Copy link
Collaborator Author

Closed by #1615

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 a pull request may close this issue.

3 participants