Skip to content

Refactor getenv access using constructor - #9

Open
bcremer wants to merge 1 commit into
dnoegel:masterfrom
bcremer:refactor-environment-access-alternative
Open

Refactor getenv access using constructor#9
bcremer wants to merge 1 commit into
dnoegel:masterfrom
bcremer:refactor-environment-access-alternative

Conversation

@bcremer

@bcremer bcremer commented Oct 24, 2014

Copy link
Copy Markdown
Contributor

Another alternative to PR #6 and #7 including the idea of @bobthecow.

@bcremer bcremer changed the title Refactor getenv access Refactor getenv access using constructor Oct 24, 2014
Comment thread src/Xdg.php Outdated

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Possibly add an explicit string cast to $env[$key] to mirror the values we'll be getting back from getenv?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

$this->env is never declared, so it's a public property.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@bobthecow

Copy link
Copy Markdown
Collaborator

👍 (of course, I'm probably biased)

@bcremer
bcremer force-pushed the refactor-environment-access-alternative branch from 2fdc5f5 to 84deaaf Compare October 24, 2014 07:11
@bcremer

bcremer commented Oct 24, 2014

Copy link
Copy Markdown
Contributor Author

Included CR feedback from @bobthecow

@GrahamCampbell

Copy link
Copy Markdown
Contributor

Any news on this @dnoegel, @bobthecow, @bcremer?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants