Chore: public structs with initializers #594
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…e of having all fields as internal in a public model;
JeneaVranceanu
commented
Jul 19, 2022
|
|
||
| public init(fromBlock: Block?, toBlock: Block?, | ||
| public init(fromBlock: Block? = nil, | ||
| toBlock: Block? = nil, |
There was a problem hiding this comment.
Combined two inits into one. Doesn't break anything.
JeneaVranceanu
commented
Jul 19, 2022
| @@ -11,6 +11,12 @@ public struct ICAP { | |||
| public var asset: String | |||
| public var institution: String | |||
| public var client: String | |||
|
|
|||
| public init(asset: String, institution: String, client: String) { | |||
There was a problem hiding this comment.
This struct is not used anywhere in the project so I assumed it should be used in other projects that use web3swift as a dependency. Thus, this struct must provide an initializer.
JeneaVranceanu
commented
Jul 19, 2022
| let info: DirectDebitInfo | ||
| let epoch: BigUInt | ||
| public let info: DirectDebitInfo | ||
| public let epoch: BigUInt | ||
| } |
There was a problem hiding this comment.
Related to both of the structs above: no use of having public structs with internal inaccessible variables. Marked all as public.
An initializer is not added as these structs are not to be initialized by other developers. These models are used to decode responses.
SwiftyJacek
added a commit
to SwiftyJacek/web3swift
that referenced
this pull request
Aug 11, 2022
commit 31b75de Merge: 3ec8b24 da1517d Author: SwiftyJacek <95341310+SwiftyJacek@users.noreply.github.com> Date: Thu Aug 11 12:02:36 2022 +0200 Merge branch 'skywinder:develop' into develop commit da1517d Merge: a0c7265 4312a71 Author: Yaroslav <yaroslav.yashin@me.com> Date: Wed Jul 20 19:48:46 2022 +0700 Merge branch 'master' into develop Dropping all frozen dependencies. commit 4312a71 Author: Yaroslav <yaroslav.yashin@me.com> Date: Tue Jul 19 18:02:28 2022 +0700 Update cocoapods version commit dff6562 Author: Yaroslav <yaroslav.yashin@me.com> Date: Tue Jul 19 18:01:55 2022 +0700 Freeze dependencies versions commit b8de0b6 Merge: e269630 a0c7265 Author: Yaroslav <yaroslav.yashin@me.com> Date: Tue Jul 19 17:27:12 2022 +0700 Merge pull request web3swift-team#595 from skywinder/develop develop to master merge commit a0c7265 Merge: ce1c62f a9461c9 Author: Yaroslav <yaroslav.yashin@me.com> Date: Tue Jul 19 16:58:48 2022 +0700 Merge pull request web3swift-team#594 from JeneaVranceanu/chore/extensions-with-initializers Chore: public structs with initializers commit a9461c9 Author: Jenea Vranceanu <jeneavranceanu@gmail.com> Date: Tue Jul 19 11:26:23 2022 +0300 fix: added default initializer for EthereumParameters; commit 647a0dc Author: Jenea Vranceanu <jeneavranceanu@gmail.com> Date: Tue Jul 19 11:23:40 2022 +0300 fix: added default initializer for TransactionOptions; commit dfa34e7 Author: Jenea Vranceanu <jeneavranceanu@gmail.com> Date: Tue Jul 19 11:23:21 2022 +0300 chore: DirectDebitInfo and DirectDebit fields made public - no purpose of having all fields as internal in a public model; commit 01f3a86 Author: Jenea Vranceanu <jeneavranceanu@gmail.com> Date: Tue Jul 19 11:21:50 2022 +0300 chore: ICAP initializer added; commit 3d7caf7 Author: Jenea Vranceanu <jeneavranceanu@gmail.com> Date: Tue Jul 19 11:20:47 2022 +0300 chore: EventFilter initializers combined;
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added initializers to public structs that had none.
By default, structs have initializers that are usable within the project because these initializers have an
internalaccess modifier.That initializer is obviously inaccessible when some library is used as a dependency in a different project.
Example of initializers available in 3rd party project for
EthereumParameters(default internal initializer is not visible):Duplicating the answer to the question from Discord about why this is merged into
develop(just for the sake of clarity).Because that (the issue) is in
developbranch and I cannot initEthereumParametersandTransactionOptionseasily becauseinits are internal.Previously in
developbranchTransactionOptionshadpublic init() {}andEthereumParameterswasn't even existing.With EIP1559 that changed:
public init() {}forTransactionOptionsand noinitforEthereumParameterswithoutTransactionOptions.This is currently the only way to initialize
TransactionOptionswhich is a requirement for initializer ofEthereumParameters.As an alternative developers could use
TransactionOptions.defaultOptionsbut then they have to set all values tonilin the returnedTransactionOptionsobject that they are not intending to use in order forpublic func merge(_ otherOptions: TransactionOptions?) -> TransactionOptionsto work as expected.All of the changes introduced with EIP1559 work perfectly well within the boundaries of the web3swift.
The only thing that was missing are public initializers for a few structs to make it possible to initialize these structs in 3rd party projects.