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

feat: release notes #283

Merged
merged 13 commits into from
Oct 12, 2024
Merged

feat: release notes #283

merged 13 commits into from
Oct 12, 2024

Conversation

doprz
Copy link
Collaborator

@doprz doprz commented Oct 7, 2024

Features

  • Add scripts to for changelog logic
  • Add git utils
  • Add PatchNotesPopup.tsx with GFM rendering + respective storybook file

Chore

  • Update packages

image


This change is Reviewable

@doprz doprz marked this pull request as ready for review October 8, 2024 03:46
@doprz doprz requested review from sghsri and Razboy20 and removed request for sghsri October 8, 2024 03:55
@DereC4 DereC4 self-requested a review October 8, 2024 04:42
Copy link
Member

@Razboy20 Razboy20 left a comment

Choose a reason for hiding this comment

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

All in all, like how it looks! Couple changes in addition to the comments left in the code:

  • Features should be placed before Bug fixes.
  • Each commit/PR should also include the author. Perhaps should be limited to only PR contributions? Open to discussion.
  • Style wise, header could probably be a bit larger, and there is too much padding at the top. Also, maybe we might want to use *color* but not sure.

src/views/components/common/PatchNotesPopup.tsx Outdated Show resolved Hide resolved
src/views/components/common/PatchNotesPopup.tsx Outdated Show resolved Hide resolved
@doprz
Copy link
Collaborator Author

doprz commented Oct 8, 2024

All in all, like how it looks! Couple changes in addition to the comments left in the code:

  • Features should be placed before Bug fixes.
  • Each commit/PR should also include the author. Perhaps should be limited to only PR contributions? Open to discussion.
  • Style wise, header could probably be a bit larger, and there is too much padding at the top. Also, maybe we might want to use color but not sure.

I'm using angular as the preset but I can also use: angular, atom, codemirror, conventionalcommits, ember, eslint, express, jquery or jshint

@doprz doprz requested a review from Razboy20 October 9, 2024 00:11
Copy link
Member

@Razboy20 Razboy20 left a comment

Choose a reason for hiding this comment

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

Looks much better!

Couple things:

  • Could you add a "Ok" button in orange
  • Have the 2.0.0-beta4 changelog title render as an actual dialog title rather than as part of the description. Not sure what the best way to do this is as it's included in the markdown; if not easily possible, style the h1 to match the default dialog title styling.
  • Double check that text stylings match component styles.
  • close button to the top right? (I don't recall whether we have a good method for the close button right now, so you can decide against this)

scripts/generateChangelog.ts Show resolved Hide resolved
@doprz
Copy link
Collaborator Author

doprz commented Oct 9, 2024

Looks much better!

Couple things:

  • Could you add a "Ok" button in orange
  • Have the 2.0.0-beta4 changelog title render as an actual dialog title rather than as part of the description. Not sure what the best way to do this is as it's included in the markdown; if not easily possible, style the h1 to match the default dialog title styling.
  • Double check that text stylings match component styles.
  • close button to the top right? (I don't recall whether we have a good method for the close button right now, so you can decide against this)
  • There's no need for a confirmation button with this dialog as it's just to show the patch notes.
  • The title is built into the CHANGELOG.md file

Copy link
Member

@DereC4 DereC4 left a comment

Choose a reason for hiding this comment

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

@doprz doprz requested a review from Razboy20 October 10, 2024 01:58
Copy link
Member

@Razboy20 Razboy20 left a comment

Choose a reason for hiding this comment

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

Getting close!!!

src/stories/components/PatchNotesPopup.stories.tsx Outdated Show resolved Hide resolved
@doprz doprz requested a review from Samathingamajig October 12, 2024 21:49
Copy link
Collaborator

@Samathingamajig Samathingamajig left a comment

Choose a reason for hiding this comment

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

LDTM

@doprz doprz dismissed Razboy20’s stale review October 12, 2024 22:05

Fixed changes requested in review and approved by Sam

@doprz doprz merged commit bd17e33 into main Oct 12, 2024
16 of 17 checks passed
@doprz doprz deleted the feature/release-notes branch October 12, 2024 22:05
@DereC4
Copy link
Member

DereC4 commented Oct 12, 2024

oh FINALLY

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants