Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Feat/compiler #483
base: main
Are you sure you want to change the base?
Feat/compiler #483
Changes from all commits
95131df
53b5a0b
1f3b3f4
240aedc
3d6f8be
49075d7
f9dab02
029f124
d07a587
3a69078
b541d22
ab1e0c9
59f7507
730f72e
d61c6ca
2fb01eb
81193e2
04d720f
1511c94
c40bfdd
32b91f7
d612816
5716258
aee5ec7
fec5790
3c1185a
af250f2
cf2c04c
ecadfb2
be5142a
1b9a348
afbf33b
faef67d
dd918cb
ae65cc5
694dbcd
6bd99b8
d67729e
375001d
a170adf
aa8a8ca
e8e9dd8
4a51caf
ae414f2
771c2cd
d92b385
4ae5997
d5c7f27
f1c9ea3
560f043
1c0c9ab
2aba5fb
d27f391
ae9be57
1317635
06432ba
46998c6
4970648
8030509
c05caa9
074b74a
f4d54d8
c5d8243
dbf7d80
8681e06
6c5ec41
98f696e
12edbb1
1dfc0ce
70cbcec
30f1b9c
ce31d3a
bf7acd0
179a39c
3e35604
7604ef5
773cf3d
7c154b9
4bcce0a
8b71fdb
b573d00
7924123
8afba76
2dbf34d
a68a248
a2620ec
d41d95e
078f95b
520ce31
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Nit) empty blank line, and a couple others around the project.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the rule here? Can we add this to prettier?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please ignore the piece about prettier
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, unfortunately I don't think prettier supports it at this time. I believe Rome might? But I haven't looked into it closely.
Broadly, separate top-level and class-level function declarations by one blank line. Top level and class level properties and variables can be grouped without blank lines between them "so that it looks good" - I know that's not a great rule, but there's room for a bit of aesthetics. "Keep similar things together" is the principle there, but not super strict.
But for the rule - every function should have a blank line before it (or before its docblock, if it has a comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this what we are looking for?
https://stackoverflow.com/questions/52772097/how-to-add-new-line-after-and-before-method-declarations-in-classes-using-pretti
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, it's a part of eslint, not prettier, that's probably why I couldn't find it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's agree that we can create another PR and standardize this across the whole codebase
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. Please do your best in the meantime to keep this as tidy as you can.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me know if you want me to change something. Otherwise I will leave it as it is
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this change need a change in the README?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've already changed the README. Both of these libraries implement the same api/interfaces
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This library already has a ton of comments in typescript sources so I don't know if makes sense to add anything else in terms of documentation