Skip to content

whitebophir: init at 1.7.0 (package and service) #85842

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.

Sign up for GitHub

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
Merged
merged 2 commits into from
Jan 13, 2021
Merged

whitebophir: init at 1.7.0 (package and service) #85842

merged 2 commits into from
Jan 13, 2021
+1,610 −0

Conversation

iblech
Copy link
Contributor

@iblech iblech commented Apr 23, 2020

Motivation for this change

There are many online collaborative whiteboard websites; this pull request packages one which I particularly like. It's simple but not trivial, easily hackable, and free.

This pull request currently targets a specific Git commit of the upstream repository, as the most recent release would require a ugly postInstall hook to fix up hard-coded paths in the source. The package derivation is loosely based on the derivation for cryptpad.

This is my first service addition, hence I'd appreciate feedback. Is there some addition you'd want to see before you deem it fit for merging?

Things done
  • 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.

Sorry, something went wrong.

@iblech iblech requested a review from infinisil as a code owner April 23, 2020 03:41
@ofborg ofborg bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 8.has: package (new) This PR adds a new package 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10 labels Apr 23, 2020
@iblech
Copy link
Contributor Author

iblech commented May 4, 2020

I just bumped this to the most recent version, 1.4.0. :-)

@iblech iblech changed the title whitebophir: init at 1.1.0 (package and service) [RFC] whitebophir: init at 1.4.0 (package and service) May 4, 2020
@iblech iblech changed the title whitebophir: init at 1.4.0 (package and service) whitebophir: init at 1.5.0 (package and service) May 17, 2020
@iblech
Copy link
Contributor Author

iblech commented May 17, 2020

Bumped to the new version 1.5.0. :-) @infinisil, did you get around reviewing this pull request? I'm happy to incorporate any changes :-)

@iblech iblech mentioned this pull request May 26, 2020
Closed
aanderse
aanderse requested changes May 26, 2020
View reviewed changes
Copy link
Member

@aanderse aanderse left a comment

Choose a reason for hiding this comment

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

Just a quick review of the module (I don't review node packages, not familiar with it). Looking good 👍

Is the idea to put this behind a reverse proxy or something?

@iblech
Copy link
Contributor Author

iblech commented May 28, 2020

Just a quick review of the module (I don't review node packages, not familiar with it). Looking good

Thank you for your encouragement and for for taking the time to review this pull request! I believe I adressed all your concerns. :-)

Is the idea to put this behind a reverse proxy or something?

Yes, exactly. Originally I wanted to include example nginx configuration settings in the documentation, but figured that I'd first wait for a review in order to resolve all concerns in one go.

@aanderse
Copy link
Member

Thanks for the clarification. Let's find someone to review the package and then you can add the documentation. Great work!

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review-may-2019/3032/171

@iblech iblech changed the title whitebophir: init at 1.5.0 (package and service) whitebophir: init at 1.6.0 (package and service) Jun 28, 2020
@iblech iblech changed the title whitebophir: init at 1.6.0 (package and service) whitebophir: init at 1.6.4 (package and service) Oct 23, 2020
@iblech iblech requested a review from aanderse October 23, 2020 12:27
@iblech iblech changed the title whitebophir: init at 1.6.4 (package and service) whitebophir: init at 1.6.5 (package and service) Jan 6, 2021
@iblech
Copy link
Contributor Author

iblech commented Jan 6, 2021

Just a heads up that I'm still using this package and service on a regular basis and that it seems to work fine. I'll appreciate any guidance on what has to be done in order for this pull request to get merged. :-)

@iblech iblech force-pushed the patch-whitebophir branch from 4053cb4 to 0f9d950 Compare January 11, 2021 14:38
@iblech iblech changed the title whitebophir: init at 1.6.5 (package and service) whitebophir: init at 1.7.0 (package and service) Jan 11, 2021
@iblech
Copy link
Contributor Author

iblech commented Jan 11, 2021

Just a couple hours ago, whitebophir 1.7.0 was released. I took the opportunity of bumping to rebase against current master.

@iblech iblech force-pushed the patch-whitebophir branch from 0f9d950 to 1f4bbf7 Compare January 11, 2021 14:50
infinisil
infinisil requested changes Jan 12, 2021
View reviewed changes
@iblech iblech force-pushed the patch-whitebophir branch from 1f4bbf7 to 2dc2781 Compare January 12, 2021 21:44
@iblech
Copy link
Contributor Author

iblech commented Jan 12, 2021

@infinisil: Thank you for your thorough review, very much appreciated. I believe I implemented all of your suggestions in the new force-push. :-)

@infinisil
Copy link
Member

Looking good!

@infinisil
Copy link
Member

infinisil commented Jan 12, 2021 •
edited
Loading

Oh one last thing: Organize the commits as:

  • An initial commit for the package (with commit message "whitebophir: init at 1.7.0")
  • Another commit for the NixOS module (with commit message "nixos/whitebophir: init")

(with git rebase -i HEAD~10)

@iblech iblech force-pushed the patch-whitebophir branch from 2dc2781 to 46292d7 Compare January 13, 2021 00:25
@iblech
Copy link
Contributor Author

iblech commented Jan 13, 2021

@infinisil Just did so. :-)

@infinisil infinisil merged commit 0cd5058 into NixOS:master Jan 13, 2021
@infinisil
Copy link
Member

Nice work!

@iblech
Copy link
Contributor Author

iblech commented Jan 13, 2021

Super awesome, thank you for handholding and merging! :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@infinisil infinisil

@aanderse aanderse

Assignees
No one assigned
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@iblech @aanderse @nixos-discourse @infinisil