-
-
Notifications
You must be signed in to change notification settings - Fork 15k
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
antlr4_13: init at 4.13.0 and set as default #238543
Conversation
Result of 2 packages failed to build:
55 packages built:
|
nativeBuildInputs = [ | ||
setuptools | ||
]; | ||
|
||
# in 4.9, test was renamed to tests |
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 know this is not strictly related to your patch. Could you explain why is this cd test*
not cd tests
? Is this expression meant to be compatible with multiple antlr versions? If not, the asterisk
could be removed.
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 think the comment pretty much has explained it, so the folder is named test
pre 4.9, but tests
since 4.9.
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 think the comment pretty much has explained it, so the folder is named
test
pre 4.9, buttests
since 4.9.
@NickCao what I tried to propose, is that perhaps because this expression is used with only a single antlr4.runtime.cpp.src
, the asterisk could be removed.
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.
But it can (and is) used with other versions of antlr, see pkgs/servers/baserow/default.nix
for example.
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.
But it can (and is) used with other versions of antlr, see
pkgs/servers/baserow/default.nix
for example.
I see, than in that case, let's write this detail in the comment, something like:
# in 4.9, test was renamed to tests | |
# We use an asterisk because this expression is used also for old antlr | |
# versions, where there the tests directory is `tests` and not `test`. See | |
# e.g in package `baserow`. |
Looks pretty good so far! I very much appreciate PRs that make the latest version the default version. Could you please run another |
4e5a5ba
to
b80aa60
Compare
Result of 4 packages failed to build:
72 packages built:
|
fairseq is broken on master: https://hydra.nixos.org/eval/1798993?filter=fairseq |
Good to merge when CI is green. |
Description of changes
Things done
sandbox = true
set innix.conf
? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)