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

Add JsLIGO support #5908

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

melwyn95
Copy link

@melwyn95 melwyn95 commented May 24, 2022

Description

Checklist:

@melwyn95 melwyn95 marked this pull request as ready for review May 24, 2022 16:53
@melwyn95 melwyn95 requested a review from a team as a code owner May 24, 2022 16:53
@melwyn95
Copy link
Author

@pewulfman @lildude @Alhadis can you review this?

@lildude
Copy link
Member

lildude commented May 25, 2022

Usage of the .jsligo extension is too low at the moment with six users contributing all but 58 files.

I'm happy to accept the other changes if you want to split the change of grammar from the addition of the new extension.

@lildude
Copy link
Member

lildude commented May 25, 2022

Can you please also update the OP to link to the source of the sample and explicitly stated the license of the file.

@melwyn95
Copy link
Author

Can you please also update the OP to link to the source of the sample and explicitly stated the license of the file.

I did not completely understand, do you mean I need to add license information to the provided sample like
(MIT License Copyright (c) 2020 LigoLANG SASU ... ) ?

@lildude
Copy link
Member

lildude commented May 25, 2022

Nope. Just fill in this part of the template that you've left empty:

I have included a real-world usage sample for all extensions added in this PR:
Sample source(s):
[URL to each sample source, if applicable]
Sample license(s):

@melwyn95
Copy link
Author

Usage of the .jsligo extension is too low at the moment with six users contributing all but 58 files.

I'm happy to accept the other changes if you want to split the change of grammar from the addition of the new extension.

I'll move this PR to Draft state for now, we are expecting the usage of JsLIGO to increase in the near future.

In a few months once we have enough usage of .jsligo I'll make this PR read, Thanks

@melwyn95 melwyn95 marked this pull request as draft May 25, 2022 07:14
@melwyn95
Copy link
Author

@lildude
Copy link
Member

lildude commented May 25, 2022

Yes. That's removing the biggest contributors so we can get a better idea of overall usage as GitHub's search is struggling at the moment. See #5756 for the temporary criteria were using.

@pewulfman
Copy link
Contributor

@lildude I am surprise about the user list that you removed from the query. I mean I understand that you remove ligolang but what about the others ? can you elaborate ?

What would be the minimum number of users and code files required to accept the PR ? The guideline says 200 user/repo

@lildude
Copy link
Member

lildude commented May 25, 2022

I'll start with this question...

What would be the minimum number of users and code files required to accept the PR ? The guideline says 200 user/repo

The 200 unique :user/:repo combination requirement is temporarily suspended in favour of the requirements detailed in #5756. .jsligo doesn't meet either of those requirement yet.

With that in mind, I'll answer:

@lildude I am surprise about the user list that you removed from the query. I mean I understand that you remove ligolang but what about the others ? can you elaborate ?

This is answered in #5756:

If particular users are showing a high proportion of the results, I'll manually filter out those users using -user: to reduce their impact on my assessment.

To further elborate, I removed those just to demonstrate that of the current 1542 files on GitHub, six users are responsible for 1504 of them. This suggests the usage on GitHub is primarily isolated to a few users and not an a good indication of wide-spread adoption yet which is what we're aiming for.

@melwyn95
Copy link
Author

melwyn95 commented Mar 10, 2023

Hey @lildude It's been around a year since I raised this PR, I see now there are 237 files according to this search url.

I am not able to find the number of users using .jsligo files the search url says 106M, Do you have some internal tool with which you can find the number of users?

Also is the current usage of .jsligo enough for this PR to be included into linguist (If yes I'll resolve the merge conflicts and make this PR ready)

Thanks.

@lildude
Copy link
Member

lildude commented Mar 10, 2023

Hey @lildude It's been around a year since I raised this PR, I see now there are 237 files according to this search url.

That's still very low, especially considering a repo can have multiple .jsligo files and as such isn't likely to meet our original requirements... even the temporarily relaxed requirements aren't be met.

Using the new search (it is still indexing repos from most recently active to least recently active so won't include everything yet) path:*.jsligo returns ~1.1k files. If we filter out just the two most prominent users as before path:*.jsligo -user:emacsmirror -user:ligolang
things drop quite significantly to 470 files. If we filter out all the users as before path:*.jsligo -user:emacsmirror -user:ligolang -user:tezos-checker -user:marigold-dev -user:smart-chain-fr -user:DiningPhilosophersCo we're down to 159 files.

I am not able to find the number of users using .jsligo files the search url says 106M, Do you have some internal tool with which you can find the number of users?

I used to use a script but this is no longer possible due to problems with the old code search and the reason for the change in process as documented in #5756

So things are still not popular enough for inclusion.

@melwyn95
Copy link
Author

Cool, Thanks for your response @lildude

@Laucans
Copy link

Laucans commented Oct 2, 2023

Hi @lildude,

Checking for this topic.
Do we have an idea how indexation works on Github ?

Looks like your request with extension got partial result for example this search result to 0 code, but this repo got jsligo file since 6 month.

Looks like I triggered manually indexation with path:*.jsligo repo:Laucans/Intezos-tutorial-1 :
image

But is the search request relevant ? Are you using another way to extract the number of distinct :user/:repo now ?

@lildude
Copy link
Member

lildude commented Oct 2, 2023

@Laucans No, I don't know how indexation works. This is maintained by a completely different team but I understand things will take time to index since we switched to using the new search functionality as active repositories are being prioritised with less active repos slowly being added as they're accessed or as the backlog is processed.

But is the search request relevant ? Are you using another way to extract the number of distinct :user/:repo now ?

The search is still relevant (it needs slight tweaking to exclude forks) and I'm still using the same temporary approach I previously referenced as there still isn't an easy way to calculate the distinct repos.

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

Successfully merging this pull request may close these issues.

None yet

4 participants