Skip to content

workflow: functionality changes release notes addition #933

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

Conversation

0xc0170
Copy link
Contributor

@0xc0170 0xc0170 commented Jan 31, 2019

Every functionality change must contain release notes text addition. This will be
gating functionality change integration.

A user does not need to go through PRs to see what has changed, read the section:
"Functionality changes" in the release notes.

Ready for reviews

cc @ARMmbed/mbed-os-maintainers

We could get some examples from current funcitonal changes and how release notes should look like. I quickly found one: ARMmbed/mbed-os#9277

Every functionality change must contain release notes text addition. This will be
gating functionality change integration.

A user does not need to go through PRs to see what has changed, read the section:
"Functionality changes" in the release notes.
Copy link
Contributor

@NirSonnenschein NirSonnenschein left a comment

Choose a reason for hiding this comment

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

look promising

@@ -98,6 +98,12 @@ Any change in the functionality, it can be adding a new feature, adding a new me

A feature contribution contains a new API, capability or behavior. It does not break backward compatibility with existing APIs, capabilities or behaviors. New feature contributions are very welcome in Mbed OS. However, because they add capability to the codebase, it's easy for a new feature to introduce bugs and a support burden. The introduction of new features should also come with documentation, majority of targets support and comprehensive test coverage proving the correctness of the feature per the documentation. Feature PRs are treated cautiously, and new features require a new minor version for the codebase. Features are candidates for feature releases.

Every functionality change pull request must contain release notes section addition to describe the change to users.
Copy link
Contributor

Choose a reason for hiding this comment

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

must contain release notes section -> must contain a release notes section
I would also propose a title for this section
so it would be:
must contain a release notes section called "Important changes"

"important changes" is a placeholder actual name still needs to be defined

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed, adding "Release notes" section for now

Every functionality change pull request must contain release notes section addition to describe the change to users.

It must contain:
- description of changes with impact analysis
Copy link
Contributor

Choose a reason for hiding this comment

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

you might want to give examples for "description of changes" and "impact analysis" as developers can interpret these in different ways.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I split them into two points

Copy link
Contributor

@NirSonnenschein NirSonnenschein left a comment

Choose a reason for hiding this comment

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

approved, with one small note

It must contain:
- brief description of changes introduced
- impact analysis - identify components affected, what are potential consequences for users, why do we need this
- migration guidance: before and after (good to include code snippets to illustrate)
Copy link
Contributor

Choose a reason for hiding this comment

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

small note: in migration guidance I suggest saying it needs to specify all the actions that the user needs to take (should be self explanatory, but still).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

+1, I rephrased it

@0xc0170 0xc0170 force-pushed the dev_functionalitychange_releasenotes branch from 4cfd9b0 to 237b127 Compare January 31, 2019 12:27
- brief description of changes introduced
- impact analysis - identify components affected, what are potential consequences for users, why do we need this
- migration guidance: actions for updating the current code. Good to include code snippets to illustrate, before/after

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe add a note here t hat this section will be used in official release notes ? or it's clear to reader

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed, a note is useful

Copy link
Member

@bulislaw bulislaw left a comment

Choose a reason for hiding this comment

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

Should we rewrite the "Functionality change" to talk both about new functionality but also about changing functionality. The whole page should be reviewed by docs team btw.

@@ -98,6 +98,13 @@ Any change in the functionality, it can be adding a new feature, adding a new me

A feature contribution contains a new API, capability or behavior. It does not break backward compatibility with existing APIs, capabilities or behaviors. New feature contributions are very welcome in Mbed OS. However, because they add capability to the codebase, it's easy for a new feature to introduce bugs and a support burden. The introduction of new features should also come with documentation, majority of targets support and comprehensive test coverage proving the correctness of the feature per the documentation. Feature PRs are treated cautiously, and new features require a new minor version for the codebase. Features are candidates for feature releases.

Every functionality change pull request must contain a release notes section called "Release notes" to describe the changes to users.
Copy link
Member

Choose a reason for hiding this comment

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

Every pull request changing functionality? @AnotherButler

Copy link
Contributor Author

@0xc0170 0xc0170 Feb 1, 2019

Choose a reason for hiding this comment

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

fixed in efc2e0b

Copy link
Contributor

Choose a reason for hiding this comment

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

Should also apply for 'Breaking Changes'

@@ -98,6 +98,13 @@ Any change in the functionality, it can be adding a new feature, adding a new me

A feature contribution contains a new API, capability or behavior. It does not break backward compatibility with existing APIs, capabilities or behaviors. New feature contributions are very welcome in Mbed OS. However, because they add capability to the codebase, it's easy for a new feature to introduce bugs and a support burden. The introduction of new features should also come with documentation, majority of targets support and comprehensive test coverage proving the correctness of the feature per the documentation. Feature PRs are treated cautiously, and new features require a new minor version for the codebase. Features are candidates for feature releases.

Every functionality change pull request must contain a release notes section called "Release notes" to describe the changes to users.
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if this paragraph keeps the spirit of the previous ones. The first and second paragraph talks exclusively about adding new functionality. Your paragraph below talks more about changing existing functionality.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I rephrased it to "Every pull request changing or adding functionality" - we want to capture additions but also changes

@0xc0170
Copy link
Contributor Author

0xc0170 commented Feb 1, 2019

Example for ARMmbed/mbed-os#9277 (@deepikabhavnani can help me to fix wording here - will edit).


Release notes

Add MBED_TRAP_ERRORS_ENABLED to develop profile - enables to catch printf in ISR in develop profile. These errors were silent in develop profile.
This change might result in the final binary size increase.


^^ this section would be part of pull request, script can fetch it and add it to the release notes.

@bulislaw
Copy link
Member

bulislaw commented Feb 1, 2019

Uhm, I like the idea of less manual work, but it also means we need to be validating it during code review. It would be good to have our release notes grammatically correct and describes the change properly (with analysis of impact and migration steps when it makes sense).

@0xc0170
Copy link
Contributor Author

0xc0170 commented Feb 1, 2019

@AnotherButler This is very important change to be included in the process as soon as possible. Please review

@deepikabhavnani
Copy link

Example for ARMmbed/mbed-os#9277 (@deepikabhavnani can help me to fix wording here - will edit).

Corrected a bit, but still @AnotherButler should verify

Release notes
Added MBED_TRAP_ERRORS_ENABLED flag to develop profile - Traps on RTX errors and invalid operations in ISR context enabled in develop profile. These errors were silent in develop profile.
This change might result in the final binary size increase.

Edit file, mostly for complete sentences and parallelism.
It must contain:

- A brief description of changes introduced.
- An impact analysis: components affected, potential consequences for users and reasons for the addition or change.
Copy link
Contributor

Choose a reason for hiding this comment

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

Query: Can we use a word other than "impact" here? It's technically imprecise language. ("Impact" implies hitting something, such as impacted teeth or a meteor impact.)

Copy link
Contributor Author

@0xc0170 0xc0170 Feb 4, 2019

Choose a reason for hiding this comment

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

OK, what other synonym could be used? I've checked some, could not find anything similar to this one.

I found usually "impact analysis" in some software journals. See for instance https://www.guru99.com/impact-analysis-software-testing.html

@0xc0170
Copy link
Contributor Author

0xc0170 commented Feb 5, 2019

@AnotherButler Fixed impact as proposed, I would still go with as it was. An analysis of effects works as well

@adbridge
Copy link
Contributor

adbridge commented Feb 5, 2019

Uhm, I like the idea of less manual work, but it also means we need to be validating it during code review. It would be good to have our release notes grammatically correct and describes the change properly (with analysis of impact and migration steps when it makes sense).

For functionality changes and breaking changes where a 'Release notes' we should add docs team as reviewers just for that section.

Add requested note to clarify content will be used in the official release note.
@AnotherButler
Copy link
Contributor

Do we know why Travis is failing?

@0xc0170
Copy link
Contributor Author

0xc0170 commented Feb 7, 2019

Travis having networking issues, restarted the build. It happens from time to time for other projects as well, restarting helps

@0xc0170
Copy link
Contributor Author

0xc0170 commented Feb 7, 2019

@AnotherButler All green, lets merge this in and let us know once it's in, we will bookmark this in the docs to share with the teams

@AnotherButler AnotherButler merged commit e754bc7 into ARMmbed:development Feb 8, 2019
AnotherButler pushed a commit that referenced this pull request Feb 8, 2019
Apply changes from PR #933 to add functionality changes and some copy edits.
@0xc0170 0xc0170 deleted the dev_functionalitychange_releasenotes branch February 11, 2019 09:34
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 this pull request may close these issues.

9 participants