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

ink! analyzer (phase 2) - milestone 4 #1097

Merged
merged 1 commit into from
Jan 19, 2024

Conversation

davidsemakula
Copy link
Contributor

Milestone Delivery Checklist

  • The milestone-delivery-template.md has been copied and updated.
  • This pull request is being made by the same account as the accepted application.
  • I have disclosed any and all sources of reused code in the submitted repositories and have done my due diligence to meet its license requirements.
  • In case of acceptance, an invoice must be submitted and the payment will be transferred to the Polkadot/fiat account provided in the application.
  • The delivery is according to the Guidelines for Milestone Deliverables.

Link to the application pull request: w3f/Grants-Program#1904 < please fill this in with the PR number of your application.

@davidsemakula
Copy link
Contributor Author

It looks like the "Google Sheet Update" action is hitting some kind of limit (or bug) https://github.com/w3f/Grant-Milestone-Delivery/actions/runs/7377042382/job/20070544925

@keeganquigley keeganquigley self-assigned this Jan 11, 2024
@keeganquigley
Copy link
Contributor

Thanks @davidsemakula you can disregard the error, we added it in manually. I will get started on this today!

@davidsemakula
Copy link
Contributor Author

Hi @keeganquigley 👋

So I just submitted milestone 6 as well.
I'm mentioning here because it may make sense to review them together because in terms of manual testing (via VS Code), milestone 6 is exactly the same as this milestone combined with milestone 3 (which you already reviewed - PR and evaluation - including manual testing via VS Code).

On the automated testing side, my default testing instructions run unit and integration tests for both the semantic-analyzer (ink-analyzer crate) and language server (ink-lsp-server crate), so the review will also be pretty much identical for both milestones from your perspective.

I thought I'd point this out since it could save some time on both sides 🙂

Lastly, if you decide to review them together, the lsp-server-v0.2.22 tag/release for the language server in milestone 6 is exactly the same commit as the analyzer-v0.8.20 tag/release for the semantic analyzer. So you can clone using either tag/release for local testing (and we can update this delivery to mention the analyzer-v0.8.20 release instead of the analyzer-v0.8.17 release).

@keeganquigley
Copy link
Contributor

Thanks @davidsemakula apologies for the delay, I should have some notes for you tomorrow.

@keeganquigley
Copy link
Contributor

Great work as usual! I did end up reviewing them together, you can find my final eval here.

@keeganquigley keeganquigley merged commit 9e02d83 into w3f:master Jan 19, 2024
5 of 6 checks passed
Copy link

🪙 Please fill out the invoice form in order to initiate the payment process. Thank you!

@davidsemakula davidsemakula deleted the ink-analyzer branch January 19, 2024 20:18
@davidsemakula
Copy link
Contributor Author

@keeganquigley Awesome! Thanks for the thorough review(s) and positive feedback 🙂.

The VS Code extension install base is also growing steadily at about ~220 unique installs now (roughly +40 new unique installs since the last review) 🚀

@davidsemakula
Copy link
Contributor Author

I've submitted the invoice 🙂

@RouvenP
Copy link

RouvenP commented Jan 26, 2024

hi @davidsemakula we just sent the payment

@davidsemakula
Copy link
Contributor Author

Hi @RouvenP,
Received ... thanks!

Just in case you missed it, there's also a second delivery/invoice that was approved the same day as this one
(see #1110)

@RouvenP
Copy link

RouvenP commented Jan 26, 2024

@davidsemakula thanks for confirming. Yes, we're aware - this one is still in the process and will most likely be paid out next Friday

@davidsemakula
Copy link
Contributor Author

@RouvenP Gotcha ... thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants