Skip to content

Add support for SPRING_QUIET environment variable. #674

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

Merged
merged 1 commit into from
Sep 20, 2022
+22 −2
Merged

Add support for SPRING_QUIET environment variable. #674

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
## Next Release

* Add support for `SPRING_QUIET` environment variable.

## 4.0.0

* Stop requiring `set` before bundler can select the proper version. This could result in
Expand Down
8 changes: 8 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,12 @@ a command runs:
Spring.quiet = true
```

You can also set the initial state of the `quiet` configuration option to true
by setting the `SPRING_QUIET` environment variable before executing Spring.
This is useful if you want to set quiet mode when invoking the Spring executable
in a subprocess, and cannot or prefer not to set it programmatically
via the `Spring.quiet` option in `~/.spring.rb` or the app's `config/spring.rb`.

### Environment variables

The following environment variables are used by Spring:
Expand All @@ -412,6 +418,8 @@ The following environment variables are used by Spring:
the long-running Spring server process. By default, this is related to
the socket path; if the socket path is `/foo/bar/spring.sock` the
pidfile will be `/foo/bar/spring.pid`.
* `SPRING_QUIET` - If set, the initial state of the `Spring.quiet`
configuration option will default to `true`.
* `SPRING_SERVER_COMMAND` - The command to run to start up the Spring
server when it is not already running. Defaults to `spring _[version]_
server --background`.
Expand Down
7 changes: 6 additions & 1 deletion lib/spring/configuration.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@

module Spring
class << self
attr_accessor :application_root, :quiet
attr_accessor :application_root
attr_writer :quiet

def gemfile
require "bundler"
Expand Down Expand Up @@ -52,6 +53,10 @@ def project_root_path
@project_root_path ||= find_project_root(Pathname.new(File.expand_path(Dir.pwd)))
end

def quiet
@quiet ||= ENV.key?('SPRING_QUIET')
Copy link
Contributor

Choose a reason for hiding this comment

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

It short-circuits in the config reader method via ||= instead of ||, allowing it to be set back to false even after spring starts, if this is necessary.

Correct me if I'm wrong, but at first glance it looks like that it won't be possible to set it back to false after setting it to true once, like:

class DummySpring
  class << self
    attr_writer :quiet

    def quiet
      @quiet ||= ENV.key?('SPRING_QUIET') 
    end
  end
end

ENV["SPRING_QUIET"] = "lol"
DummySpring.quiet # => true
DummySpring.quiet = false
DummySpring.quiet # => still true because `ENV.key?('SPRING_QUIET') ` gets checked again as long as `@quiet` is falsey

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nvasilevski That's a good point. You could set it back to false via the attr_writer, but not via the ENV var.

But I guess I assumed that you wouldn't try to use the ENV var to change it while an app is executing - if you want to do that, just use the attr_writer.

In other words, the ENV var is only needed when you don't have access to modify the code or config at all, but still want to turn quiet mode on,

This is also explained by the new documentation:

You can also set the initial state of the quiet configuration option to true
by setting the SPRING_QUIET environment variable before executing Spring.

Note it says "initial state" and "...before executing Spring".

So, I guess I think this is OK as is, especially given the documentation.

WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

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

You could set it back to false via the attr_writer, but not via the ENV var.

In my example I'm showing that it's not possible to reset it using writer after setting it once to true because of how ||= works. If we want to support that, reader will need to be implemented as:

def quiet
    return @quiet if defined(@quiet)

    @quiet = ENV.key?('SPRING_QUIET')
end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In my example I'm showing that it's not possible to reset it using writer after setting it once to true because of how ||= works.

Ah, gotcha, you're right.

So ideally we can come up with a case where it can be changed from true to false or vice versa, via either the attr_writer or the ENV var.

I'll try to come up with a change that does that and then ping you to review it.

Thanks for your help and attention!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I got busy/lazy and never returned to finish up this thread.

But I see this got merged now, which is great!

This thread describes a minor bug which should be fixed eventually, so I'll keep it on my TODO list - near the bottom, which realistically means "probably never", but hope springs eternal ;)

Copy link
Contributor

Choose a reason for hiding this comment

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

No worries, there was an additional commit on top of this PR to remove the memoization and make ENV variable to take precedence over a falsey @quiet value

1908710

end

private

def find_project_root(current_dir)
Expand Down
7 changes: 6 additions & 1 deletion test/support/acceptance_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,12 +125,17 @@ def without_gem(name)
assert_success app.spring_test_command, stderr: "Running via Spring preloader in process"
end

test "does not tell the user that Spring is being used when used automatically via binstubs but quiet is enabled" do
test "does not tell the user that Spring is being used when quiet is enabled via Spring.quiet" do
File.write("#{app.user_home}/.spring.rb", "Spring.quiet = true")
assert_success "bin/rails runner ''"
refute_output_includes "bin/rails runner ''", stderr: 'Running via Spring preloader in process'
end

test "does not tell the user that Spring is being used when quiet is enabled via SPRING_QUIET ENV var" do
assert_success "SPRING_QUIET=true bin/rails runner ''"
refute_output_includes "bin/rails runner ''", stderr: 'Running via Spring preloader in process'
end

test "raises if config.cache_classes is true" do
config_path = app.path("config/environments/development.rb")
config = File.read(config_path)
Expand Down