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

Change grammar source for Genero language and rename #6632

Merged
merged 4 commits into from
Dec 11, 2023

Conversation

sebflaesch
Copy link
Contributor

@sebflaesch sebflaesch commented Dec 11, 2023

Description

For .4gl and .per files, using vendor's official repository for syntax highlighting: FourjsGenero/GeneroFgl.tmbundle

Checklist:

@lildude
Copy link
Member

lildude commented Dec 11, 2023

Please update the template explaining why you are removing the original samples. As mentioned before, we only remove samples if they're blatantly wrong such that they're either complete rubbish, are useless "Hello world" types or are for a completely different language as these all affect the classifier. The code samples don't have to work in isolation, they just need to be real world samples people have actually used as some point.

@lildude
Copy link
Member

lildude commented Dec 11, 2023

Oh yes, and add an explanation for the renames too please. This helps users who notice the change understand why it has happened when they find this PR.

Copy link
Member

@lildude lildude left a comment

Choose a reason for hiding this comment

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

Looks like the license file from the grammar wasn't cached or you forgot to commit it.

If you can't find the changed file, please run this command to download it and then add it to this PR:

bundle exec licensed cache -c vendor/licenses/config.yml

@lildude lildude changed the title Issue 6629: Use FourjsGenero/GeneroFgl.tmbundle for Genero language Change grammar source for Genero language and rename Dec 11, 2023
@sebflaesch
Copy link
Contributor Author

sebflaesch commented Dec 11, 2023

Oh...
It's my first PR so I will have to learn about how to update a PR.

The command for the license did not gave me any comment.
Is the required file something like
vendor/licenses/git_submodule/GeneroFgl.tmbundle.dep.yml
?

I do not know how to update the PR, should I just commit / push new changes in my fork/branch and ref to it here?

I have updated the description of the PR.

@sebflaesch
Copy link
Contributor Author

sebflaesch commented Dec 11, 2023

BTW we are the software vendor of Genero 4gl/per so about samples removal, please trust us, and consider the existing samples as "blatantly wrong such that they're either complete rubbish".

@sebflaesch sebflaesch requested a review from lildude December 11, 2023 15:34
@lildude
Copy link
Member

lildude commented Dec 11, 2023

I see you've found the file and worked out how to add it to the PR (commit to your fork and push and it updates the PR).

Regarding:

  • This is to fit with the names we use in our VSCode extension.

Is this how users and the company refer to the names of the languages in discussions, usage etc? I ask as this is what appears on the sidebar on repos and in the search results and most companies prefer the official names of the languages to be reflected and users also prefer the names they generally use.

@sebflaesch
Copy link
Contributor Author

sebflaesch commented Dec 11, 2023

Is this how users and the company refer to the names of the languages in discussions, usage etc?

Yes. We use different names, actually:

  • Genero BDL - marketing ( may change in the future! ;-) )
  • Genero FGL - technical
  • Genero 4GL - technical

Last one "Genero 4gl" is the best match, as it corresponds to the .4gl file extension.

Note that there are other compilers/languages that use the .4gl and .per file extensions:

  • "Informix 4GL" ("Genero 4gl" is a superset of the legacy "Informix 4gl" language)
  • "Querix 4gl" (one of our competitors)

So you can easily distinguish "Genero 4gl" from "Informix 4gl" and "Querix 4gl"

There should be no language name just defined as "4GL" or "4gl"

Copy link
Member

@lildude lildude left a comment

Choose a reason for hiding this comment

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

Great! Thanks for all the explanations etc. Looks good to me now. I aim to make a release tomorrow morning #6627 and this will be included.

@lildude lildude added this pull request to the merge queue Dec 11, 2023
Merged via the queue into github-linguist:master with commit a45d988 Dec 11, 2023
5 checks passed
@lildude
Copy link
Member

lildude commented Dec 12, 2023

Oooof:

rm Gemfile.lock && script/bootstrap
==> Initialising submodules…
Error updating vendor/grammars/GeneroFgl.tmbundle
fatal: remote error: upload-pack: not our ref 08c18d5226822a1ebdc21d3f8454a12c19e080ae
fatal: Fetched in submodule path 'vendor/grammars/GeneroFgl.tmbundle', but it did not contain 08c18d5226822a1ebdc21d3f8454a12c19e080ae. Direct fetching of that commit failed.
Submodule path 'vendor/grammars/syntax': checked out 'ced984baed78a85834dbbd1724d3e600ba50ef8d'
✘ git st
## feature/Terraform-templates
 M script/bootstrap
 M vendor/grammars/GeneroFgl.tmbundlegit submodule update --init -- vendor/grammars/GeneroFgl.tmbundle/
fatal: remote error: upload-pack: not our ref 08c18d5226822a1ebdc21d3f8454a12c19e080ae
fatal: Fetched in submodule path 'vendor/grammars/GeneroFgl.tmbundle', but it did not contain 08c18d5226822a1ebdc21d3f8454a12c19e080ae. Direct fetching of that commit failed.

I'll see if I can fix this as part of the new release.

@lildude
Copy link
Member

lildude commented Dec 12, 2023

Ignore ☝️ I think it might have been something with the branch which had some other odd things going on. Things are looking good on master.

@sebflaesch
Copy link
Contributor Author

@lildude, I was wondering how/when the new changes for Genero will enter in action on other github repos...
I guess you need to "release" a new linguist version to make it happen?
Is the process documented somewhere?

@lildude
Copy link
Member

lildude commented Dec 12, 2023

Yes, I need to make a release and deploy it to github.com. I briefly document the steps in the release PR, in this case #6627, which I check off as each happens.

I've hit a bit of a snag with the syntax highlighter which needs the service owners to look in to (they're all in the US) so there's going to be a delay to the next steps.

@github-linguist github-linguist locked as resolved and limited conversation to collaborators Jun 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants