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

ISSUE#405 dev programs rework #409

Merged
merged 18 commits into from
Oct 2, 2024
Merged

ISSUE#405 dev programs rework #409

merged 18 commits into from
Oct 2, 2024

Conversation

frankiebee
Copy link
Collaborator

@frankiebee frankiebee commented Aug 5, 2024

Related Issue(s)

closes #405

Testing

  • Unit tests added/updated
  • Integration tests added/updated

Checklist

  • I have performed a self-review of my code.
  • I have commented my code.
  • I have updated documentation, where necessary.

Copy link

github-actions bot commented Aug 5, 2024

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@frankiebee
Copy link
Collaborator Author

I have read the CLA Document and I hereby sign the CLA

@frankiebee frankiebee marked this pull request as ready for review August 6, 2024 15:08
@frankiebee
Copy link
Collaborator Author

frankiebee commented Aug 6, 2024

DONT merge yet wanted to see CLA bot would rerun after draft

@frankiebee frankiebee changed the title Frankie/i#405programs [DONT MERGE] Frankie/i#405programs Aug 6, 2024
@frankiebee frankiebee changed the title [DONT MERGE] Frankie/i#405programs [DONT MERGE] ISSUE#405 dev programs rework Aug 6, 2024
Entropy-Machine-CI added a commit to entropyxyz/.github that referenced this pull request Aug 6, 2024
@frankiebee frankiebee changed the title [DONT MERGE] ISSUE#405 dev programs rework ISSUE#405 dev programs rework Aug 19, 2024
src/programs/dev.ts Outdated Show resolved Hide resolved
Copy link
Collaborator

@mixmix mixmix left a comment

Choose a reason for hiding this comment

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

Looks good!
Requested changes that I think are important are marked with 🔥

  • 🔥 CHANGELOG needed
  • 🔥 put some check in to catch inevitable accidental misuse of new `get
  • 🔥 fix failing test

src/registration/index.ts Outdated Show resolved Hide resolved
tests/programs-dev.test.ts Show resolved Hide resolved
tests/programs-dev.test.ts Outdated Show resolved Hide resolved
tests/programs-dev.test.ts Outdated Show resolved Hide resolved
tests/programs-dev.test.ts Show resolved Hide resolved
tests/programs-dev.test.ts Outdated Show resolved Hide resolved
@frankiebee
Copy link
Collaborator Author

  • 🔥 fix failing test

What test is failing?

@mixmix
Copy link
Collaborator

mixmix commented Aug 21, 2024

The one in this comment - #409 (comment)
it's failing but commented out

@frankiebee
Copy link
Collaborator Author

ahh missed that in the comment.

Co-authored-by: mix irving <mix@protozoa.nz>
Copy link
Collaborator

@mixmix mixmix left a comment

Choose a reason for hiding this comment

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

My only 🔥 is I'm pretty concerned about giving up on Schema Parsing.

I would probably throw an error, and if you want, attach the data did some out ok to that error?

CHANGELOG.md Show resolved Hide resolved
* @property {ArrayBuffer} bytecode - The bytecode of the program.
* @property {unknown} [interfaceDescription] - Optional. The configuration interface of the program.
* @property {string} deployer - The address of the deployer of the program.
* @property {number} refCounter - The reference count for the program.
*/

// interfaceDescription needs better design and another type other than 'unknown'
export interface ProgramInfo {
export interface ProgramInterface {
Copy link
Collaborator

Choose a reason for hiding this comment

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

before release, lets see if we can get this exported for use in CLI / other apps

src/programs/dev.ts Show resolved Hide resolved
src/programs/dev.ts Show resolved Hide resolved
src/programs/dev.ts Outdated Show resolved Hide resolved
src/utils/index.ts Outdated Show resolved Hide resolved
tests/programs-dev.test.ts Outdated Show resolved Hide resolved
tests/programs-dev.test.ts Outdated Show resolved Hide resolved
@frankiebee frankiebee merged commit e2cf2ac into main Oct 2, 2024
3 checks passed
@frankiebee frankiebee deleted the frankie/i#405programs branch October 2, 2024 23:33
@github-actions github-actions bot locked and limited conversation to collaborators Oct 2, 2024
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.

[anchor] PROGRAMS
2 participants