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

Jump to bottom

codimd: build with nodejs-12_x instead of nodejs-10_x #97463

Merged
merged 1 commit into from Sep 9, 2020
Merged

codimd: build with nodejs-12_x instead of nodejs-10_x #97463

merged 1 commit into from Sep 9, 2020

Conversation

WilliButz
Copy link
Member

Motivation for this change

#97377

Things done

The codimd test is now also exposed via passthru.tests

  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

fixes #97377

@WilliButz
codimd: build with nodejs-12_x instead of nodejs-10_x
46e3949 
The codimd test is now exposed via passthru.tests
@WilliButz WilliButz added the 9.needs: port to stable A PR needs a backport to the stable release. label Sep 8, 2020
@WilliButz WilliButz requested a review from FRidh September 8, 2020 18:20
@ofborg ofborg bot requested review from globin and Ma27 September 8, 2020 18:40
Ma27
Ma27 reviewed Sep 8, 2020
View reviewed changes
Copy link
Member

@Ma27 Ma27 left a comment

Choose a reason for hiding this comment

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

LGTM.

@WilliButz have you checked if the test actually caught the issue? If not we should probably improve it (not in this PR though, this would be out of scope IMHO).

@WilliButz
Copy link
Member Author

@Ma27 yes the test fails for sqlite, you should be able to verify it on the current master or release branch :)

@Ma27 Ma27 merged commit 1b344f2 into NixOS:master Sep 9, 2020
@Ma27
Copy link
Member

Ma27 commented Sep 9, 2020

Ported to 20.09 as 3265417.

@Ma27 Ma27 added 8.has: port to stable A PR already has a backport to the stable release. and removed 9.needs: port to stable A PR needs a backport to the stable release. labels Sep 9, 2020
@WilliButz WilliButz deleted the codimd/fix-sqlite/node12 branch September 11, 2020 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Ma27 Ma27 Ma27 approved these changes

@FRidh FRidh Awaiting requested review from FRidh

@globin globin Awaiting requested review from globin

Assignees
No one assigned
Labels
8.has: port to stable A PR already has a backport to the stable release. 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 10.rebuild-linux: 1 11.by: package-maintainer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

services.codimd: Error: Please install sqlite3 package manually
2 participants
@WilliButz @Ma27