Skip to content
This repository has been archived by the owner on Dec 8, 2023. It is now read-only.

Implement GetGenericPortMappingEntry function #43

Merged
merged 1 commit into from
Jan 19, 2020

Conversation

maufl
Copy link
Contributor

@maufl maufl commented Jan 16, 2020

I implemented the required functions for the GetGenericPortMappingEntry function. I'm not sure whether the new PortMappingEntry struct is a good idea as a return value, or whether all fields are ok (lease_duration could for example be a Duration).

This branch is based on the branch of my other PR, so I'd hold of reviewing it for now.

@sbstp
Copy link
Owner

sbstp commented Jan 17, 2020

Thanks for the other MR! Do you want to rebase this branch on master and then I'll merge & release?

@maufl maufl force-pushed the implement-get-generic-port-mapping-entry branch from c953c07 to 643ecdd Compare January 18, 2020 11:06
@maufl maufl force-pushed the implement-get-generic-port-mapping-entry branch from 643ecdd to ce884c2 Compare January 18, 2020 11:09
}
}

impl std::error::Error for GetGenericPortMappingEntryError {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I noticed that description is deprecated and Display should be used instead and returning None for cause/source is the default so this implementation contains nothing.

@maufl
Copy link
Contributor Author

maufl commented Jan 18, 2020

Done. Do you think introducing the PortMappingEntry struct is a good idea?

@sbstp
Copy link
Owner

sbstp commented Jan 18, 2020

Done. Do you think introducing the PortMappingEntry struct is a good idea?

Sure, I don't see any issues with it.

@maufl
Copy link
Contributor Author

maufl commented Jan 19, 2020

Then I think you can merge it.

@sbstp sbstp merged commit bc436ea into sbstp:master Jan 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants