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.

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

json2csv: init at 5.0.0 #85115

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

json2csv: init at 5.0.0 #85115

wants to merge 1 commit into from

Conversation

glittershark
Copy link
Member

I did this using the generate.sh shell script which appeared to be the way to go, but I'm surprised at how many changes there are here... let me know if I should've done something different.

Motivation for this change

Using the bin from this package in a project

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.

@glittershark
Copy link
Member Author

/marvin opt-in

@marvin-mk2 marvin-mk2 bot added the marvin This PR was reviewed by Marvin, a discontinued bot: https://github.com/timokau/marvin-mk2 label Jun 16, 2020
@marvin-mk2
Copy link

marvin-mk2 bot commented Jun 16, 2020

Hi! I'm an experimental bot. My goal is to guide this PR through its stages, hopefully ending with a merge. The stages are

  • needs_review, if the author considers this PR ready
  • needs_work if the PR in its current form is not ready yet. Maybe the reviewer requested changes, there is an ongoing discussion or you are waiting for upstream feedback.
  • needs_merge can be set by reviewers who do not have merge permission but would merge this PR if they could.

Anybody can switch the current status with a comment of the form /status <new_status_here>.

Feedback and contributions to this bot are appreciated.

@glittershark
Copy link
Member Author

/status needs_review

@marvin-mk2 marvin-mk2 bot added the needs_reviewer (old Marvin label, do not use) label Jun 16, 2020
@timokau
Copy link
Member

timokau commented Jun 16, 2020

Merge conflict.
/status needs_work

@marvin-mk2 marvin-mk2 bot added awaiting_changes (old Marvin label, do not use) and removed needs_reviewer (old Marvin label, do not use) labels Jun 16, 2020
@marvin-mk2 marvin-mk2 bot added awaiting_reviewer (old Marvin label, do not use) and removed awaiting_changes (old Marvin label, do not use) labels Jul 8, 2020
@timokau timokau mentioned this pull request Jul 8, 2020
Open
@timokau
Copy link
Member

timokau commented Jul 8, 2020

I'm not familiar with / comfortable reviewing node packages. It seems weird that the two files node-packages-v12.nix and node-packages-v13.nix were created completely from scratch and didn't previously exist. There's probably something wrong.

@svanderburg, are you willing to review? (Identified with git shortlog -n pkgs/development/node-packages/).

@glittershark
Copy link
Member Author

yeah... I thought it was weird too - the generate.sh script spat them out

@marvin-mk2 marvin-mk2 bot added needs_reviewer (old Marvin label, do not use) and removed awaiting_reviewer (old Marvin label, do not use) labels Jul 11, 2020
@marvin-mk2 marvin-mk2 bot requested a review from timokau July 14, 2020 02:49
@marvin-mk2 marvin-mk2 bot added awaiting_reviewer (old Marvin label, do not use) and removed needs_reviewer (old Marvin label, do not use) labels Jul 14, 2020
@timokau
Copy link
Member

timokau commented Jul 14, 2020

Looks like nobody is willing to review node packages :/ Going to the next git shortlog -n pkgs/development/node-packages entry, @Ma27 could you maybe have a look?

@marvin-mk2 marvin-mk2 bot added needs_reviewer (old Marvin label, do not use) and removed awaiting_reviewer (old Marvin label, do not use) labels Jul 17, 2020
@marvin-mk2 marvin-mk2 bot requested a review from ryantm July 23, 2020 04:19
@marvin-mk2 marvin-mk2 bot removed the needs_reviewer (old Marvin label, do not use) label Jul 23, 2020
@marvin-mk2 marvin-mk2 bot added the needs_reviewer (old Marvin label, do not use) label Jul 26, 2020
@Mic92
Copy link
Member

Mic92 commented Jul 30, 2020 •
edited
Loading

We have unified it into one node-packages.nix file now. I can merge this if merge conflicts are resolved.

@marvin-mk2 marvin-mk2 bot added awaiting_reviewer (old Marvin label, do not use) and removed needs_reviewer (old Marvin label, do not use) labels Jul 30, 2020
@Mic92
Copy link
Member

Mic92 commented Jul 30, 2020

/status awaiting_changes

@marvin-mk2 marvin-mk2 bot added awaiting_changes (old Marvin label, do not use) and removed awaiting_reviewer (old Marvin label, do not use) labels Jul 30, 2020
@glittershark
Copy link
Member Author

ugh, that means I need to run the generate script again 😭

there's got to be a better way

@marvin-mk2 marvin-mk2 bot added awaiting_reviewer (old Marvin label, do not use) and removed awaiting_changes (old Marvin label, do not use) labels Jul 30, 2020
@Mic92
Copy link
Member

Mic92 commented Jul 30, 2020

It's ugly yes, here is an upstream discussion svanderburg/node2nix#192
I would be helpful if node2nix would do parallel request also... svanderburg/node2nix#181

@marvin-mk2 marvin-mk2 bot added needs_reviewer (old Marvin label, do not use) and removed awaiting_reviewer (old Marvin label, do not use) labels Aug 8, 2020
@marvin-mk2 marvin-mk2 bot requested a review from fgaz August 8, 2020 12:10
@marvin-mk2 marvin-mk2 bot added awaiting_reviewer (old Marvin label, do not use) and removed needs_reviewer (old Marvin label, do not use) labels Aug 8, 2020
@fgaz
Copy link
Member

fgaz commented Aug 8, 2020

Still conflicting
/status awaiting_changes

@marvin-mk2 marvin-mk2 bot added awaiting_changes (old Marvin label, do not use) and removed awaiting_reviewer (old Marvin label, do not use) labels Aug 8, 2020
@ryantm ryantm added 2.status: merge conflict This PR has merge conflicts with the target branch and removed 2.status: merge conflict This PR has merge conflicts with the target branch labels Oct 3, 2020
@stale
Copy link

stale bot commented Jun 7, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 7, 2021
@timokau timokau removed the marvin This PR was reviewed by Marvin, a discontinued bot: https://github.com/timokau/marvin-mk2 label Apr 16, 2022
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Apr 16, 2022
@timokau timokau removed the awaiting_changes (old Marvin label, do not use) label Apr 16, 2022
@timokau
Copy link
Member

timokau commented Apr 16, 2022

Hi!

The marvin-mk2 bot is now discontinued. I have removed the relevant tags from this PR. If you still need someone to look at it, one option would be to ask in this discourse thread.

I am posting this notice to all open PRs with the marvin tag. Please understand that following all of these discussions would take too much time, so I will unsubscribe from this PR unless I have already been involved in it before this message.

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Nov 2, 2022
@wegank wegank marked this pull request as draft March 20, 2024 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@timokau timokau Awaiting requested review from timokau

@ryantm ryantm Awaiting requested review from ryantm

@fgaz fgaz Awaiting requested review from fgaz

Assignees
No one assigned
Labels
2.status: merge conflict This PR has merge conflicts with the target branch 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 11-100
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@glittershark @timokau @Mic92 @fgaz @ryantm