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 documention minor issues #2250

Merged
merged 5 commits into from
Aug 7, 2023
Merged

Fix documention minor issues #2250

merged 5 commits into from
Aug 7, 2023

Conversation

AmirRezaM75
Copy link
Contributor

No description provided.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 4, 2023

Changes in this PR will be published to the following url to try(check status of TypeSpec Pull Request Try It pipeline for publish status):
Playground: https://cadlplayground.z22.web.core.windows.net/prs/2250/

Website: https://cadlwebsite.z1.web.core.windows.net/prs/2250/

Comment on lines +116 to +119
model PetId {
@path petId: int32;
}

Copy link
Contributor Author

@AmirRezaM75 AmirRezaM75 Aug 4, 2023

Choose a reason for hiding this comment

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

Model is used in line 126. having it here, make it more clear

@@ -70,7 +70,7 @@ emit:
- emitter1
```

in `<my-pkg>/tspconfig.yaml`, enable `emitter2` using the options specified in the parent `tspconfig.yaml
in `<my-pkg>/proj2/tspconfig.yaml`, enable `emitter2` using the options specified in the parent `tspconfig.yaml
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In my opinion the second example is not necessary and has been covered above.

Copy link
Member

Choose a reason for hiding this comment

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

that's fair, feel free to remove that one

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

@@ -17,7 +17,7 @@ The guidelines in this article are used in TypeSpec Core libraries. You can use
| scalar | camelCase | `scalar uuid extends string;` |
| model | PascalCase | `model Pet {}` |
| model property | camelCase | `model Pet {furColor: string}` |
| enum | PascalCase | `model Pet {furColor: string}` |
| enum | PascalCase | `enum Direction {}` |
| enum member | camelCase | `enum Direction {up, down}` |
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Although all examples mentioned here https://microsoft.github.io/typespec/language-basics/enums has PascalCase members 😕

Copy link
Member

Choose a reason for hiding this comment

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

Good catch, sounds like we need to update the other docs too. They were written before we came to an agreement for the style guide

@AmirRezaM75
Copy link
Contributor Author

@microsoft-github-policy-service agree

@timotheeguerin
Copy link
Member

@microsoft-github-policy-service agree

Hi @AmirRezaM75 thanks for the contribution, It seems like it hasn't picked up the cla agreement, I think it might be because it has the markdown quote prefix >

If you comment again without I think it should work!

@AmirRezaM75
Copy link
Contributor Author

@microsoft-github-policy-service agree

@AmirRezaM75
Copy link
Contributor Author

@timotheeguerin
I agreed with policy in dedicated comment, tell me if it doesn't work.
Does docs' compiler script runs in your CI/CD or do I have to run it locally and commit changes?
Ready for review.

@timotheeguerin
Copy link
Member

Seemed to have worked thanks! Yes the docs will get published automatically however I just noticed you updated the “latest” docs version in versioned_docs folder. Those are going to get overwritten next release(next week) by the content of docs/ would you mind making the same fixes there as well

@AmirRezaM75
Copy link
Contributor Author

Seemed to have worked thanks! Yes the docs will get published automatically however I just noticed you updated the “latest” docs version in versioned_docs folder. Those are going to get overwritten next release(next week) by the content of docs/ would you mind making the same fixes there as well

@timotheeguerin Done.

@timotheeguerin timotheeguerin merged commit ad2b2c8 into microsoft:main Aug 7, 2023
10 checks passed
@AmirRezaM75 AmirRezaM75 deleted the fix_doc branch August 7, 2023 18:36
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