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

Fix AS Migration Guide upcasting code snippet #70

Merged
merged 1 commit into from
Feb 28, 2022

Conversation

evaporei
Copy link
Contributor

At some point in the internationalization of the docs, some code got it's formatting broken.

This PR fixes this issue only for the AssemblyScript Migration Guide.

Some of the fixes include:

  • Adding semicolons to the code
  • Fixing casting examples
  • Adding deleted comments in translations
  • Fixing compiler error messages in translations

@evaporei evaporei requested a review from azf20 February 25, 2022 12:42
@evaporei evaporei self-assigned this Feb 25, 2022
@evaporei evaporei requested a review from a team as a code owner February 25, 2022 12:42
@@ -91,19 +91,19 @@ maybeValue.aMethod();
ولكن في الإصدار الأحدث ، نظرًا لأن القيمة nullable ، فإنها تتطلب منك التحقق ، مثل هذا:

```typescript
let maybeValue = load()
let maybeValue = load();
Copy link
Contributor

Choose a reason for hiding this comment

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

As mentioned on Slack, we should fix the translations on Crowdin instead of in this PR. I will take care of it when this PR is merged, but can you please revert your changes to non-English files?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@@ -91,19 +91,19 @@ maybeValue.aMethod();
However on the newer version, because the value is nullable, it requires you to check, like this:

```typescript
let maybeValue = load()
let maybeValue = load();
Copy link
Contributor

Choose a reason for hiding this comment

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

As mentioned on Slack, please run Prettier (with yarn prettier) and you'll see that the semicolons are automatically removed.

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've removed all semicolons!

```

```typescript
// upcasting on class inheritance
class Bytes extends Uint8Array {}

let bytes = new Bytes(2) < Uint8Array > bytes // same as: bytes as Uint8Array
Copy link
Contributor

Choose a reason for hiding this comment

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

This is how Prettier will reformat these two lines of code (presumably a bug), so maybe change them to something like:

let bytes = new Bytes(2)
// `<Uint8Array>bytes` is equivalent to `bytes as Uint8Array`

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed!

Copy link
Contributor

@benface benface left a comment

Choose a reason for hiding this comment

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

See my comments above. Thank you! 💪

@evaporei evaporei removed the request for review from azf20 February 27, 2022 22:55
@evaporei
Copy link
Contributor Author

evaporei commented Feb 27, 2022

@benface all comments addressed 🙂

I've created an issue to track the missing points I've saw in this page #72

@evaporei evaporei force-pushed the otavio/fix-migration-guide-fmt branch from c6aaa69 to cc418c2 Compare February 27, 2022 22:59
@evaporei evaporei merged commit 21670e7 into main Feb 28, 2022
@evaporei evaporei deleted the otavio/fix-migration-guide-fmt branch February 28, 2022 09:10
@evaporei evaporei changed the title Fix AS Migration Guide formatting for all langs Fix AS Migration Guide upcasting code snippet Feb 28, 2022
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.

2 participants