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

Temporarily remove deprecations for existing capability API #2673

Conversation

m-Peter
Copy link
Contributor

@m-Peter m-Peter commented Jul 21, 2023

Closes onflow/vscode-cadence#400

Description

This is causing some confusion to developers, as CapCons is not yet live on Mainnet. The changes affect both AuthAccount & PublicAccount.


  • Targeted PR against master branch
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work
  • Code follows the standards mentioned here
  • Updated relevant documentation
  • Re-reviewed Files changed in the Github PR explorer
  • Added appropriate labels

@codecov
Copy link

codecov bot commented Jul 21, 2023

Codecov Report

Merging #2673 (e2a14fb) into master (181924e) will not change coverage.
The diff coverage is n/a.

❗ Current head e2a14fb differs from pull request most recent head 5cf6009. Consider uploading reports for the commit 5cf6009 to get more accurate results

@@           Coverage Diff           @@
##           master    #2673   +/-   ##
=======================================
  Coverage   78.57%   78.57%           
=======================================
  Files         338      338           
  Lines       78216    78216           
=======================================
  Hits        61456    61456           
  Misses      14474    14474           
  Partials     2286     2286           
Flag Coverage Δ
unittests 78.57% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@m-Peter m-Peter force-pushed the temporarily-remove-existing-capability-api-deprecations branch from e2a14fb to 5cf6009 Compare July 21, 2023 13:24
@turbolent
Copy link
Member

Instead of removing the deprecation annotations, the docstrings of the deprecated functions should be prefixed with the deprecation annotation depending on if the capability controller API feature flag is enabled.

@m-Peter
Copy link
Contributor Author

m-Peter commented Jul 24, 2023

I found the relevant config. However, the Go files are auto-generated from the respective definitions on the Cadence files. Which means that I can't edit the runtime/sema/authaccount.gen.go & runtime/sema/publicaccount.gen.go. Is there a way to utilize this config from a Cadence file?

Copy link
Member

@turbolent turbolent left a comment

Choose a reason for hiding this comment

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

Let's remove them for now. We can bring them back using a dedicated lint, which can depend on the checker config.

@turbolent turbolent merged commit 62d6ec8 into onflow:master Aug 2, 2023
10 of 12 checks passed
@m-Peter m-Peter deleted the temporarily-remove-existing-capability-api-deprecations branch August 9, 2023 05:57
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.

Capabilities API labeled deprecated but Capability Controllers aren't yet on m
3 participants