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

validate version bump PRs in "keyboard shortcuts" repo #1576

Open
alodahl opened this issue Jun 24, 2021 · 4 comments
Open

validate version bump PRs in "keyboard shortcuts" repo #1576

alodahl opened this issue Jun 24, 2021 · 4 comments
Labels
EASY Quick or simple task good-first-issue Simple issue for those new to the repo or open-source in general pinned

Comments

@alodahl
Copy link
Collaborator

alodahl commented Jun 24, 2021

Please test these version bump PRs locally to ensure that our package.json commands run without any new warnings or errors, and that the app behaves as expected. The UI should behave the same as the live site: https://techtonica.github.io/keyboard-shortcuts-practice/
Please let us know if we're clear to merge, or if there are issues. Gracias!

version bump PRs to review: https://github.com/Techtonica/keyboard-shortcuts-practice/pulls

@alodahl alodahl added EASY Quick or simple task good-first-issue Simple issue for those new to the repo or open-source in general labels Jun 24, 2021
@stale
Copy link

stale bot commented Sep 22, 2021

Although it may very well be a good idea, this issue has not had recent activity and is unlikely to be worked on. Consider creating your own pull request for the issue if that is feasible. If no further activity occurs, this will be closed in 14 days. Thank you for your contribution.

@danisyellis
Copy link

danisyellis commented Oct 28, 2021

Before starting this, I tested the code as it currently is.
I saw that
a) you say that "The UI should behave the same as the live site" but when testing the site after using npm startor npm run dev, the app doesn't give the "you were slower..." or "you were faster..." info like the live site does. Basically, everything works the same, but the wording is a little different.
b) running npm start:prod fails with this error: Error: Dialect needs to be explicitly supplied as of v4.0.0 at new Sequelize

I'm assuming these things are what's expected from this repo and tested the dependabot PRs with the intention to only flag problems if I saw any different behavior.
I tested all 5 dependabot PRs currently in the repo and didn't see any other issues, so I've approved each of them.

Last note: when I test dependabot PRs for my own repo, I test each one individually, merge it in, and then test the next, on the offchance that one affects the next. In this case, since I've verified them individually, and none of them are major version changes, I think they can all be merged in at once. But then there should be one last verification once they're all in!

I think you've made me a maintainer on the keyboard shortcuts repo, so if you want, I can do the merging and final verification. I just wanted to check my assumptions above before doing that.

I hope getting these version bumps verified makes it easier to work on any other issues or features you're hoping to get done :-)

@alodahl
Copy link
Collaborator Author

alodahl commented Oct 30, 2021

Thanks Dani! I'm fine with you merging things in yourself. I think that I may have messed things up as I was trying to merge "backend" into main today, so sorry if I did.

@alodahl
Copy link
Collaborator Author

alodahl commented Oct 30, 2021

If you'd like to make a tiny PR that just fixes a typo somewhere, I can approve it so this counts for Hacktoberfest

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
EASY Quick or simple task good-first-issue Simple issue for those new to the repo or open-source in general pinned
Projects
None yet
Development

No branches or pull requests

5 participants