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

chore: create script for updating RELEASE.md before PR #232

Merged
merged 6 commits into from
Dec 16, 2022

Conversation

NeilAn99
Copy link
Contributor

@NeilAn99 NeilAn99 commented Dec 9, 2022

chore: create script for updating RELEASE.md before PR

Fixes #148

Description of changes:

  • Added the prompt-sync and semver packages.
  • Created a script pr-updater.js located in the packages directory that can be run using npm run release. This script will prompt the contributor for the package changed, the type of PR, and the description(s) of changes made. Then, the script will find the corresponding RELEASE.md file and update it.

For example, the running the script:
image
Will produce the following RELEASE.md:
image

Tag a reviewer: @ayoayco

Tasks:

  • I have ran the build command to make sure apps are working: npm run build
  • I have ran the tests to make sure nothing is broken: npm run test
  • I have ran the linter to make sure code is clean: npm run lint
  • I have reviewed my own code to remove changes that are not needed

@netlify
Copy link

netlify bot commented Dec 9, 2022

Deploy Preview for astro-reactive canceled.

Name Link
🔨 Latest commit f24841c
🔍 Latest deploy log https://app.netlify.com/sites/astro-reactive/deploys/6394ee110dffab00087d5850

@netlify
Copy link

netlify bot commented Dec 9, 2022

Deploy Preview for astro-reactive-docs canceled.

Name Link
🔨 Latest commit f24841c
🔍 Latest deploy log https://app.netlify.com/sites/astro-reactive-docs/deploys/6394ee119c71ed0008417377

Comment on lines 44 to 55
console.log(`What package is changed?
1. Common
2. Form
3. Validator`);
const noChanged = prompt(": ");

console.log(`What is the type of PR?
1. Fix
2. Minor Release/Feat
3. Major Release/Feat`);
const type = prompt(": ");

Copy link
Member

Choose a reason for hiding this comment

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

I think it should check the input from the prompt before going to the next step. This should prevent someone accidentally (or intentionally) type an invalid option.

Copy link
Member

@ayoayco ayoayco Dec 9, 2022

Choose a reason for hiding this comment

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

@NeilAn99 since the bump of version will not be needed per PR, you can remove adding the version for now in your script. Just focus on the change and type. Where the type could either be "fix" or "feature"

So no need to add the big text for version but just items like:

Features
- Feature description text

Fixes
- Change description text
- dix description 2

Comment on lines 16 to 24
if (type == '1') {
version = semver.inc(formJSON.version, 'patch');
}
else if (type == '2') {
version = semver.inc(formJSON.version, 'minor');
}
else {
version = semver.inc(formJSON.version, 'major');
}
Copy link
Member

Choose a reason for hiding this comment

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

I prefer to use switch-case here since type value should be only '1', '2' and '3'. Also, this else here will let other value than '1' and '2' make a major change. 😅

Copy link
Member

@ayoayco ayoayco Dec 9, 2022

Choose a reason for hiding this comment

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

As I mentioned above we can remove this versioning as we will only bump the version when releasing, and not per PR.

However I think we want to ask the contributors if the change will break the existing examples/demo apps and if they have also ipdated them.

After typing a code change description, we ask:
"Will this code change break existing examples and/or demo applications? Y/N"

Then we can just add a text (BREAKING CHANGE) beside the code change items

Features
- Change  description text (BREAKING CHANGE)
...

Copy link
Member

@ayoayco ayoayco left a comment

Choose a reason for hiding this comment

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

Thanks @NeilAn99, this is great progress for the project. We want to get this in! Can you go through a couple of suggestions first?

@ayoayco
Copy link
Member

ayoayco commented Dec 9, 2022

Oh and one very important thing I missed: the package.json "version" property in the package needs to be updated to the same as the version that is written to the RELEASE.md.

Please ignore this comment ☝️ and focus on the first comments. Bumping versions will only be done on release, and not per PR, so I will work on this after your pr is merged @NeilAn99

@NeilAn99
Copy link
Contributor Author

I have updated the code so that it doesn't deal with versioning, added some input checking and focused on features/fixes. Now running the script will look like:
image
and will produce:
image
Multiple updates to the same package will look like:
image
Is this acceptable? @Icyscools @ayoayco

Icyscools
Icyscools previously approved these changes Dec 10, 2022
Copy link
Member

@Icyscools Icyscools left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀 However, please fix a conflict on a package.json. I think it was a line in the publish script. Make sure to use it from our latest main branch and it'll be safe to merge. 🦺

@NeilAn99
Copy link
Contributor Author

I have resolved the conflict.

@ayoayco
Copy link
Member

ayoayco commented Dec 16, 2022

Thanks @Icyscools and @NeilAn99 -- I'm merging this now! :)

@ayoayco ayoayco merged commit c736757 into astro-reactive:main Dec 16, 2022
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.

chore: create script for updating RELEASE.md before PR
3 participants