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

nix: Add argbash support + some refactors #1819

Merged
merged 17 commits into from
Apr 18, 2021

Conversation

wolfgangwalther
Copy link
Member

This adds argbash support to our nix-shell scripts. None of the current scripts use any options or flags, yet, but I'd like to do so in #1805 and #1812. All scripts now have a --help output to display the docs string we had already in place.

Also added some smaller refactors I did on the other branches already - and some new ones, too.

We still need to figure out how to add argbash auto-complete support, but we can do that later.

@wolfgangwalther
Copy link
Member Author

wolfgangwalther commented Apr 17, 2021

We still need to figure out how to add argbash auto-complete support, but we can do that later.

Did it now.

TODO:

  • Add required environment variables for some scripts via ARG_USE_ENV
  • Use ARG_POSITIONAL_DOUBLEDASH everywhere with requirement to use optional before positional arguments (improves passing arguments to cabal etc)

@wolfgangwalther
Copy link
Member Author

Unless CI tells me otherwise... I think I'm done with this for now.

Feedback very much welcome, especially from maybe just running nix-shell on this branch, then switching somewhere else and developing regularly for a bit. Might have missed something obvious.

@steve-chavez
Copy link
Member

Really cool, didn't know about argbash. I'm liking the new -h commands and how some commands automatically output the help when they require arguments and 0 are given.

The -h doesn't seem to be working on some commands though:

postgrest-release-github -h
line 25: GITHUB_TOKEN: unbound variable

postgrest-release-dockerhub -h
line 15: DOCKER_REPO: unbound variable

postgrest-release-dockerhubdescription -h
line 12: DOCKER_USER: unbound variable

@wolfgangwalther
Copy link
Member Author

wolfgangwalther commented Apr 18, 2021

Hm. I am getting the following output:

$ postgrest-release-github -h
postgrest-release-github
Usage: postgrest-release-github [-h|--help] [--] <version>
	<version>: git version tag to make release for
	-h, --help: Prints help

Environment variables that are supported:
	GITHUB_TOKEN: GitHub token.
	GITHUB_USERNAME: GitHub user name.
	GITHUB_REPONAME: GitHub repository name.

Push a new release to GitHub.

Are you sure, you are on the latest commit of this branch? The last few commits changed some stuff for the postgrest-release- commands.

@monacoremo
Copy link
Member

I love it! works great for me. That will bring our dev environment to another level!

@wolfgangwalther
Copy link
Member Author

Had a few more small changes and refactors to the nix code. Importing stuff into shell.nix is now a lot cleaner and all modules have a common interface.

Two changes, for consistency:

  • postgrest-docker-login is now postgrest-release-docker-login (same namespace as the other release tools)
  • nix-shell --arg memoryTests true is now nix-shell --arg memory true because I always forget whether it's memoryTest or memoryTests - and nix-shell does not complain.

Also fixed postgrest-push-cachix, which used system-binaries and didn't work with nix-shell --pure.

@wolfgangwalther
Copy link
Member Author

Short of CI failures, I'll leave it at that and start rebasing my other two nix PRs on this one.

@monacoremo
Copy link
Member

6d1d001 not sure about that one... 'flat is better than nested'. Should we leave it flat? Or alternatively, name the directory 'tools'?

@wolfgangwalther
Copy link
Member Author

6d1d001 not sure about that one... 'flat is better than nested'. Should we leave it flat? Or alternatively, name the directory 'tools'?

Flat is fine - until I can't find things anymore :D. It was a lot of files by now in nix/, and since they all share the same kind of interface now, I figured it made sense. Also with loadtest and changelog there's two more files coming in.

I had tools first and then wasn't sure about hsie. It's a tools, but not a script. I'd be fine with tools, though.

@monacoremo
Copy link
Member

and since they all share the same kind of interface now, I figured it made sense.

Yes, good point.

hsie is a special case, might be spun out into a separate repo at some point. Let's go with tools?

@wolfgangwalther wolfgangwalther merged commit 7ca0d46 into PostgREST:main Apr 18, 2021
@wolfgangwalther wolfgangwalther deleted the nix/argbash branch April 18, 2021 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants