-
-
Notifications
You must be signed in to change notification settings - Fork 145
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
Bump snippets
to bb00f9
#408
Bump snippets
to bb00f9
#408
Conversation
Something I've noticed, is that doing any So I've deleted the We do use yarn for this repo, but both so long as either npm or yarn update the dependencies appropriately - they should both be fine. I do recommend having |
Ah, muscle memory always leads me to |
Oof.
|
I'd drop your entire Since that's going to replicate how our CI environments will start anyway |
OK, |
Just a comment on the To a certain extent, this rises to the level of being a bug (npm/cli#5126). (I think they change it intentionally, but it'd be nice if they didn't change it at all, IMO, which was the status quo in npm 6.x that they have since changed in newer npm.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a ton for doing this, and thanks a ton for properly including the PR's that have been merged!
So, unfortunately, newer npm doesn't really play nice with (And to clarify, that is yarn v1 anywhere in the Pulsar org, also known as If you want to do just the |
So I specifically did re-do this with yarn; that was the “Fix yarn.lock” commit. But please do give it a shot and let me know if you were able to get the diff smaller. |
This comment was marked as resolved.
This comment was marked as resolved.
By all means! You've got write access to this branch. Appreciate it. |
yarn.lock diff was minimized, so the "requested change" from this review comment has been implemented
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving that this is a validly performed dependency bump.
I can also confirm this PR is bumping to the commit at the tip of the "snippets" repo's master
branch at the moment I'm writing this (bb00f909c6
), which I confirm does include at least the four PRs mentioned in this PR's description.
(Note: Some PRs were merged at "snippets" repo with merge commit messages that don't mention the PR # in the first line, but they do mention the PR number in the body text/subsequent lines of the merge commit message.)
git log --graph --oneline output (click to expand):
It's easier to see in graph view, but this output is much more readable if you run this locally, due to color-coded graph lines.
% git log --oneline --graph 388d9b6~3...origin/master
* bb00f90 (HEAD -> master, origin/master, origin/HEAD) Merge pull request #5 from Sertonix/cleanup-and-master
|\
| * 0bbbeac Merge branch 'master' into cleanup-and-master
| |\
| |/
|/|
* | 12eec6d Merge pull request #6 from pulsar-edit/fix-tests
|\ \
| * \ 3cd111d (origin/fix-tests) Merge branch 'master' into fix-tests
| |\ \
| * | | fe00fd6 Fix failing specs
* | | | 76a32e5 Fix global flag on snippet transformations
|\ \ \ \
| |_|/ /
|/| | |
| * | | c5c73c0 Update version to 1.7.0
| * | | cebceda Remove implicit `g` flag from snippet transformations
|/ / /
* | | d0c574a [feat] Add command property to Snippet entries
|\ \ \
| |/ /
|/| |
| * | 91076f5 Update .github/workflows/pulsar_test.yml
| * | 8f87140 Apply suggestions from code review
| * | 6410899 Fix spec failure on Windows
| * | 159459f Workflow attempt 2
| * | d4835d8 Testing a branch of action-pulsar-dependency
| * | 5bf4a85 Debounce the user-snippets-file-changed handler
| * | 34bd7ff Fix failing test
| * | e8c3495 Update README.md.
| * | 8133849 Add tests.
| * | d944225 Add ability to assign a snippet to a command name
| * | 29deddc Add eslint
|/ /
* | 009b767 (deedeeg/master) Fix typo
| * 4b9d45c remove coffeelint
| * 26695d8 cleanup and rename
|/
* 388d9b6 Merge pull request #2 from Sertonix/remove-fs-plus
[ . . . snipped more commits . . . ]
Alright, with two approvals on this PR I'll go ahead and merge it. Thanks @savetheclocktower for this one, we appreciate you! |
Bumps
snippets
fromfe00fd6933fa33f819d14cdd6938d538d25ba1dd
tobb00f909c6c645b173f27346875d8fa0c7af09f7
.Provides the following PRs:
pulsar-edit/snippets#10
: “Addcommand
property that registers a command name for a snippet”pulsar-edit/snippets#7
: “Remove implicitg
flag from snippet transformations”pulsar-edit/snippets#6
: “Fix failing specs”pulsar-edit/snippets#5
: “cleanup and rename”Let me know if this should be done a different way. I ran
npm install "https://github.com/pulsar-edit/snippets.git#bb00f909c6c645b173f27346875d8fa0c7af09f7"
and
npm
converted it to the syntax you see in thepackage.json
. I’m also not wild about how muchyarn.lock
got changed for such a small update.