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

Remove dist folder, closes #820 #840

Merged
merged 2 commits into from
Jan 25, 2018
Merged

Remove dist folder, closes #820 #840

merged 2 commits into from
Jan 25, 2018

Conversation

zenflow
Copy link
Member

@zenflow zenflow commented Jan 23, 2018

Note this PR is also aimed at the canary branch.

This is inevitable for the next release.

In the meantime this will help to work in Git more easily, in canary and in master too if you base your work off of canary and then rebase it onto master.

@zenflow
Copy link
Member Author

zenflow commented Jan 23, 2018

Actually @limonte what if we extended this PR with a script to make a release (a script that would implement this idea of having a dist branch #820 (comment)):

  1. ensure Git is on master branch
  2. do some sanity checks on packageJson.version (make sure it was updated)
  3. delete the dist folder (as it will conflict with the next step)
  4. switch to the dist branch
  5. merge from master
  6. run the build
  7. commit the changes in the dist folder
  8. tag the commit with v${packageJson.version}
  9. npm publish
  10. git push origin dist:dist --tags
  11. switch back to master so you can continue to work

@zenflow
Copy link
Member Author

zenflow commented Jan 23, 2018

This would mean zero changes to how Bower users use our package. We could merge it right into version 7.

@limonte limonte merged commit 3e74a42 into canary Jan 25, 2018
@limonte
Copy link
Member

limonte commented Jan 25, 2018

Actually @limonte what if we extended this PR with a script to make a release (a script that would implement this idea of having a dist branch #820 (comment)):

ensure Git is on master branch
do some sanity checks on packageJson.version (make sure it was updated)
delete the dist folder (as it will conflict with the next step)
switch to the dist branch
merge from master
run the build
commit the changes in the dist folder
tag the commit with v${packageJson.version}
npm publish
git push origin dist:dist --tags
switch back to master so you can continue to work

This would mean zero changes to how Bower users use our package. We could merge it right into version 7.

Sounds like a perfect solution which will prevent the breaking change! Please create another PR and target the canary branch. After review, we can merge the canary into master.

@limonte limonte deleted the feature/remove-dist-folder branch January 25, 2018 12:47
@zenflow
Copy link
Member Author

zenflow commented Jan 25, 2018

Sounds like a perfect solution which will prevent the breaking change!

I'm glad you like it!

After review, we can merge the canary into master.

You mean cut the next major release? We should try populating this project board! https://github.com/sweetalert2/sweetalert2/projects/1

@limonte
Copy link
Member

limonte commented Jan 25, 2018

You mean cut the next major release?

Yup. If everything goes as expected users won't have any breaking changes. Without breaking changes we don't need a major bump.

@zenflow
Copy link
Member Author

zenflow commented Jan 25, 2018

Oh, yeah. Since canary has already collected a breaking change, we can't merge it into master unless we're cutting a major release. (Which I thought about and then realized I'm crazy since we have no reason to do that right now; it just seemed fun at the time lol)

But it's OK, this branch is based off of master so I just have to add another commit and then we can just also merge it into master. And merge it into canary again.

@zenflow
Copy link
Member Author

zenflow commented Jan 25, 2018

Trying to test this before "putting it in production" will be interesting. I'll give it a --dry-run option

@zenflow
Copy link
Member Author

zenflow commented Jan 25, 2018

Skipping #s 9 and 10

@zenflow zenflow restored the feature/remove-dist-folder branch January 25, 2018 14:37
@zenflow
Copy link
Member Author

zenflow commented Jan 25, 2018

so I just have to add another commit and then we can just also merge it into master. And merge it into canary again.

Hmm.. I guess that won't work because the squash-and-merge technically creates a different commit with a different history (but identical code). I undid this merge via 8d982b2 so as not to complicate things further. I'll open a new PR targeting master, and then once it's merged I'll just do the regular "Merge branch 'master' into canary"

zenflow added a commit that referenced this pull request Jan 25, 2018
zenflow added a commit that referenced this pull request Jan 25, 2018
@limonte limonte deleted the feature/remove-dist-folder branch January 27, 2018 11:11
limonte pushed a commit that referenced this pull request Jan 29, 2018
limonte pushed a commit that referenced this pull request Jan 29, 2018
limonte pushed a commit that referenced this pull request Jan 29, 2018
* cherry-picked "Remove dist folder, closes #820 (#840)"

* Rename `config` dir to `utils`

* Rename utils.js to package-rollup.js

* Initial draft of release script (see PR #840)

* Log stack trace to console on errors in release script

* Loosen test to determine cleanWorkingTree

* Rename cleanWorkingTree -> isCleanWorkingTree
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.

None yet

2 participants