Skip to content
This repository has been archived by the owner on Oct 19, 2024. It is now read-only.

Prestwich/event no lifetime #2105

Merged
merged 2 commits into from
Feb 14, 2023
Merged

Prestwich/event no lifetime #2105

merged 2 commits into from
Feb 14, 2023

Conversation

prestwich
Copy link
Collaborator

Depends on #2082

Motivation

FunctionCall and Event both are created by Contract types via similar APIs. However, Event had a lifetime and borrowed the client, while FunctionCall cloned the client.

Solution

This PR updates Event to work as FunctionCall does.

Old Style:

  • Event<'a, M, D>

New Style:

  • Event<B, M, D>

PR Checklist

  • Added Tests
  • Added Documentation
  • Updated the changelog
  • Breaking changes

@prestwich prestwich added the enhancement New feature or request label Feb 1, 2023
@prestwich prestwich self-assigned this Feb 1, 2023
Copy link
Owner

@gakonst gakonst left a comment

Choose a reason for hiding this comment

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

supportive of the concept, the base PR shoudl be ready to go once @mattsse takes a look

@gakonst
Copy link
Owner

gakonst commented Feb 7, 2023

dependency prs merged

@prestwich prestwich force-pushed the prestwich/event-no-lifetime branch 2 times, most recently from 3ed7ab7 to 002c17a Compare February 13, 2023 18:13
@prestwich prestwich marked this pull request as ready for review February 13, 2023 18:13
Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

The elimination of lifetime is a good change imo.

@prestwich
Copy link
Collaborator Author

can't reproduce the CI failures locally :/

@prestwich
Copy link
Collaborator Author

this is ready for review. The current lint and doc failures seem to be unrelated? I can't reproduce locally either.

@DaniPopes
Copy link
Collaborator

DaniPopes commented Feb 13, 2023

this is ready for review. The current lint and doc failures seem to be unrelated? I can't reproduce locally either.

I tried compiling with the latest nightly (rustc 1.69.0-nightly (5b8f28453 2023-02-12)) and the compiler panics at random points

After a few attempts I got it to compile at least with clippy and it found the type parameter goes unused in function definition in ethers-solc: #2146

Copy link
Owner

@gakonst gakonst left a comment

Choose a reason for hiding this comment

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

Good with me, prefer generic over lifetime. CI issues unrelated. Thank you for pushing this thru James, keen to start making cracks at the new middleware little by little.

@gakonst gakonst merged commit 0c16eb9 into master Feb 14, 2023
@gakonst gakonst deleted the prestwich/event-no-lifetime branch February 14, 2023 01:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants