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

Add identifier to Snapshot #214

Merged

Conversation

hfehrmann
Copy link
Collaborator

Hi!

This MR adds an identifier parameter to pretty syntax for Snapshot

I use this feature a lot and felt it was missing.

@dogo
Copy link
Collaborator

dogo commented Nov 11, 2020

@hfehrmann thanks for your contribution, you also need to update the PrettySyntax.swift.

▸ Compiling PrettySyntax.swift
❌  /Users/distiller/Nimble-Snapshots/Nimble_Snapshots/PrettySyntax.swift:21:45: extra argument 'identifier' in call
    return Snapshot(name: name, identifier: identifier, record: false, usesDrawRect: usesDrawRect)

@dogo dogo added enhancement work-in-progress Defines a work in progress labels Nov 11, 2020
@hfehrmann
Copy link
Collaborator Author

@dogo Corrected! I couldn't actually compile the code so I made the modifications blind.

@hfehrmann hfehrmann force-pushed the feature/add-identifier-to-pretty-snapshot branch from b537b54 to 6ac4bf8 Compare November 12, 2020 01:27
@dogo
Copy link
Collaborator

dogo commented Nov 12, 2020

Great! Good job, LFTM @ashfurrow . Let's merge?

@dogo dogo removed the work-in-progress Defines a work in progress label Nov 12, 2020
Copy link
Owner

@ashfurrow ashfurrow left a comment

Choose a reason for hiding this comment

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

Looks good! Sorry for the delay, I was offline. I'll get this released in a new version today.

@ashfurrow ashfurrow merged commit 9b759d6 into ashfurrow:master Nov 23, 2020
@ashfurrow-peril
Copy link

Thanks a lot for contributing @hfehrmann! You've been invited to be a collaborator on this repo – no pressure to accept! If you'd like more information on what this means, check out the Moya contributor guidelines and feel free to reach out with any questions.

Generated by 🚫 dangerJS

@ashfurrow
Copy link
Owner

Alrigty, this was released as 9.1.0. Should be all set, thanks again!

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.

None yet

3 participants