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

Run formatter for CHANGELOG.md when generated during release #204

Closed

Conversation

josephperrott
Copy link
Member

See individual commits as this change contains a few refactors to make this easier to reason about.

Closes #197

@josephperrott josephperrott added action: merge The PR is ready for merge by the caretaker action: review The PR is still awaiting reviews from at least one requested reviewer labels Sep 4, 2021
@google-cla google-cla bot added the cla: yes label Sep 4, 2021
Copy link
Member

@devversion devversion left a comment

Choose a reason for hiding this comment

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

LGTM. Nice work on the assertion stuff. That is pretty cool!

A couple of comments on logic organization & the testing bits.

ng-dev/format/format.ts Show resolved Hide resolved
ng-dev/release/notes/cli.ts Outdated Show resolved Hide resolved
ng-dev/release/notes/release-notes.ts Outdated Show resolved Hide resolved
ng-dev/release/notes/release-notes.ts Outdated Show resolved Hide resolved
ng-dev/release/notes/release-notes.ts Outdated Show resolved Hide resolved
ng-dev/release/publish/test/test-utils/action-mocks.ts Outdated Show resolved Hide resolved
ng-dev/release/publish/test/test-utils/sandbox-testing.ts Outdated Show resolved Hide resolved
ng-dev/utils/testing/bazel-env.ts Show resolved Hide resolved
ng-dev/utils/assertion-typings.d.ts Outdated Show resolved Hide resolved
@josephperrott josephperrott force-pushed the format-changelog-on-release branch 5 times, most recently from ef8e46d to 110927e Compare September 8, 2021 18:40
…onfig function

Allow the getConfig function take in a list of assertions functions which will assert against the
retrieved config object.  Errors from the assertions will be thrown as expected.  The returned config
object will has the asserted typings.

This will allow retrieving the config object already typed, which provides a convenience for cases
where getConfig is called on intialization.
…nd checking format of files

Returning an exit code to represent the functions failure/success allows for other tooling to utilize
the functions in place, previously when process.exit was called directly, it would end the process
entirely.
…the changelog

Support prepending the release note entries to the changelog.md file. Additionally, we
try to run the formatter on the changelog file to ensure that if formatting is required
for the file it is completed.

Additionally, updating the `ng-dev release notes` command to leverage the newly created
`prependEntryToChangelog` method.
@josephperrott josephperrott force-pushed the format-changelog-on-release branch from 110927e to dbc0cd7 Compare September 8, 2021 18:52
Moves all of the references and instances of testTmpDir to be centralized to using one common symbol.
Previously it was defined in two different places and ended up being two different directories.
… to update changelogs during release.

Use the appendEntryToChangelog method to update the changelogs, this will enable us to have the logic
of managing the changelog file exist in the ReleaseNotes class rather than having the release tool
have to understand how to stitch the two together.
…that the temp directory is cleared

Creating a bootstrap script for jasmine tests allows us to ensure automatically for all
jasmine tests that the temporary directory created for the test runs is cleaned between specs.
@josephperrott josephperrott force-pushed the format-changelog-on-release branch from dbc0cd7 to 98ca7b5 Compare September 8, 2021 19:03
ng-dev/utils/testing/virtual-git-client.ts Show resolved Hide resolved
tools/jasmine/bootstrap.spec.ts Outdated Show resolved Hide resolved
tools/jasmine/bootstrap.ts Outdated Show resolved Hide resolved
deps = kwargs.pop("deps", []) + [
"//tools/jasmine:bootstrap",
# Because we don't provided a bundled script, we must ensure that the dependencies for
# bootstrap are also included as dependencies.
Copy link
Member

Choose a reason for hiding this comment

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

Aren't the transitive deps of the bootstrap target automatically unwrapped?

you could simplify this by just depending on the TS target here while still having the other filegroup for the ES5 file above in the templated args (named bootstrap_init_file or something)

Copy link
Member Author

Choose a reason for hiding this comment

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

I switched to doing so, though I don't love that we have to still have both dependencies listed here.

Copy link
Member

@devversion devversion left a comment

Choose a reason for hiding this comment

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

Nice! this turned out pretty large but it's a great refactoring with the beforeEach jasmine logic.

@devversion devversion closed this in f631e36 Sep 9, 2021
devversion pushed a commit that referenced this pull request Sep 9, 2021
…nd checking format of files (#204)

Returning an exit code to represent the functions failure/success allows for other tooling to utilize
the functions in place, previously when process.exit was called directly, it would end the process
entirely.

PR Close #204
devversion pushed a commit that referenced this pull request Sep 9, 2021
…the changelog (#204)

Support prepending the release note entries to the changelog.md file. Additionally, we
try to run the formatter on the changelog file to ensure that if formatting is required
for the file it is completed.

Additionally, updating the `ng-dev release notes` command to leverage the newly created
`prependEntryToChangelog` method.

PR Close #204
devversion pushed a commit that referenced this pull request Sep 9, 2021
…204)

Moves all of the references and instances of testTmpDir to be centralized to using one common symbol.
Previously it was defined in two different places and ended up being two different directories.

PR Close #204
devversion pushed a commit that referenced this pull request Sep 9, 2021
… to update changelogs during release. (#204)

Use the appendEntryToChangelog method to update the changelogs, this will enable us to have the logic
of managing the changelog file exist in the ReleaseNotes class rather than having the release tool
have to understand how to stitch the two together.

PR Close #204
devversion pushed a commit that referenced this pull request Sep 9, 2021
…that the temp directory is cleared (#204)

Creating a bootstrap script for jasmine tests allows us to ensure automatically for all
jasmine tests that the temporary directory created for the test runs is cleaned between specs.

PR Close #204
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Oct 10, 2021
@josephperrott josephperrott deleted the format-changelog-on-release branch August 10, 2022 19:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker action: review The PR is still awaiting reviews from at least one requested reviewer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Code formatting should be run on CHANGELOG.md during release
2 participants