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.

Sign up for GitHub

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

Jump to bottom

Env-common cleanups and dead code removal #1077

Merged
merged 18 commits into from
Sep 20, 2023
Merged

Env-common cleanups and dead code removal #1077

merged 18 commits into from
Sep 20, 2023

Conversation

graydon
Copy link
Contributor

@graydon graydon commented Sep 20, 2023 •
edited
Loading

A bunch more improvements from code review of common: convert, env, error, lib, meta:

  • Remove various sources of potential panic: calls to panic!, unreachable!, assert! or the like.
  • Weaken the recently-added From bound on EnvBase::Error, move it to a method provided by the Env, to remove the ambient impl of From for Infallible that was in common. Only the Guest needs to provide such a path.
  • Remove some other dead methods in EnvBase.
  • Remove the dead unimplemented_env and the non-wasm Guest definition.
  • Improve some comments and localize some cfgs.

Also includes a cherry pick of f03b542 that bumps the version to rc2 (this seems not to have landed on main, just the tag?)

graydon and others added 17 commits September 19, 2023 19:48
@graydon graydon requested review from sisuresh and a team as code owners September 20, 2023 04:53
@graydon graydon mentioned this pull request Sep 20, 2023
Merged
dmkozh
dmkozh reviewed Sep 20, 2023
View reviewed changes
Copy link
Contributor

@dmkozh dmkozh left a comment

Choose a reason for hiding this comment

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

nit: Could you please update the description to match the contents of the PR (I think it would be more useful even if it's just 'env-common cleanup')

soroban-env-common/src/convert.rs Show resolved Hide resolved
@graydon graydon changed the title Graydon code review 2 Env-common cleanups and dead code removal Sep 20, 2023
@graydon graydon added this pull request to the merge queue Sep 20, 2023
Merged via the queue into main with commit 44cba6a Sep 20, 2023
9 checks passed
@graydon graydon deleted the graydon-code-review-2 branch September 20, 2023 20:38
github-merge-queue bot pushed a commit to stellar/rs-soroban-sdk that referenced this pull request Sep 21, 2023
@graydon
Graydon env code review fallout 2 (#1093)
dd1cd5c 
Adapting SDK to stellar/rs-soroban-env#1077 --
needs to have its refs updated when that merges.
github-merge-queue bot pushed a commit to stellar/rs-soroban-sdk that referenced this pull request Sep 21, 2023
@graydon
Adapt to minor env changes from code review (#1093)
7619dc4 
Adapting SDK to stellar/rs-soroban-env#1077 --
needs to have its refs updated when that merges.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dmkozh dmkozh dmkozh approved these changes

@sisuresh sisuresh Awaiting requested review from sisuresh sisuresh is a code owner

@leighmcculloch leighmcculloch Awaiting requested review from leighmcculloch

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@graydon @dmkozh