Skip to content

Set up environment variables using the Maven plugin #12800

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
Closed
wants to merge 1 commit into from
Closed

Set up environment variables using the Maven plugin #12800

wants to merge 1 commit into from

Conversation

@nosan
Copy link
Contributor

@nosan nosan commented Apr 8, 2018

see gh-11065

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Apr 8, 2018
@philwebb philwebb added the for: team-attention An issue we'd like other members of the team to review label Apr 9, 2018
@philwebb philwebb added type: enhancement A general enhancement and removed for: team-attention An issue we'd like other members of the team to review status: waiting-for-triage An issue we've not yet triaged labels Apr 11, 2018
@philwebb philwebb added this to the Backlog milestone Apr 11, 2018
@MeeTooFirst1
Copy link

MeeTooFirst1 commented Apr 16, 2018

YEAH!!

spencergibb
spencergibb reviewed Apr 16, 2018
View reviewed changes
...ls/spring-boot-maven-plugin/src/it/run-envargs/src/main/java/org/test/SampleApplication.java Outdated
}


private static class Assert {
Copy link
Member

@spencergibb spencergibb Apr 16, 2018

Choose a reason for hiding this comment

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

This is a strange way to do this. Why not a method with 2 parameters?

Copy link
Contributor Author

@nosan nosan Apr 16, 2018

Choose a reason for hiding this comment

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

First of all, thank you for your comment.
The reason why I did this is encapsulation System.getenv('key') and just do it similar to
the Assertions.assertThat(envKey).isEqualTo(val).
Of course, it could be

void assert(String envKey, String expectedValue) {
 ///
}

But in this particular case, it doesn't matter, but still if you have any concerns we can discuss this.

snicoll
snicoll requested changes Apr 17, 2018
View reviewed changes
Copy link
Member

@snicoll snicoll left a comment

Choose a reason for hiding this comment

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

We're considering adding something more in line with what the surefire plugin does (see #10741 for System properties with systemPropertyVariables). I'd like this PR to follow the same lead with a environmentVariables map.

Could you please update the PR in that direction?

@snicoll snicoll changed the title gh-11065. Set up environment variables using the Maven plugin. Set up environment variables using the Maven plugin Apr 17, 2018
@snicoll snicoll mentioned this pull request Apr 17, 2018
Closed
@snicoll snicoll self-assigned this Apr 17, 2018
@nosan
Copy link
Contributor Author

nosan commented Apr 17, 2018

sure, I will do this later.

@MeeTooFirst1
Copy link

MeeTooFirst1 commented Apr 17, 2018 via email

@philwebb
Copy link
Member

philwebb commented Apr 17, 2018

@MeeTooFirst1 PR stands for pull-request but please don't spam our issue tracker with questions.

@nosan nosan force-pushed the gh-11065 branch 2 times, most recently from 0e2a9a3 to 0cbbd54 Compare April 17, 2018 22:10
@nosan
Copy link
Contributor Author

nosan commented Apr 17, 2018 •
edited
Loading

@snicoll done :)

@snicoll snicoll modified the milestones: Backlog, 2.1.0.M1 Apr 24, 2018
snicoll pushed a commit that referenced this pull request Apr 24, 2018
@nosan @snicoll
Add support for environment variables
95f7e3c 
See gh-12800
@snicoll snicoll closed this in 40b7e02 Apr 24, 2018
snicoll added a commit that referenced this pull request Apr 24, 2018
@snicoll
Merge pull request #12800 from nosan:gh-11065
9ed169a 
* pr/12800:
  Polish "Add support for environment variables"
  Add support for environment variables
@snicoll
Copy link
Member

snicoll commented Apr 24, 2018

@nosan thanks a lot for your contribution. This is now merged in master with a polish commit.

@nosan nosan deleted the gh-11065 branch May 24, 2018 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@snicoll snicoll snicoll requested changes

+1 more reviewer

@spencergibb spencergibb spencergibb left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@snicoll snicoll

Labels

type: enhancement A general enhancement

Projects

None yet

Milestone

2.1.0.M1

Development

Successfully merging this pull request may close these issues.

6 participants

@nosan @MeeTooFirst1 @philwebb @snicoll @spencergibb @spring-projects-issues