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

Lps 123057 #1226

Closed
wants to merge 6 commits into from
Closed

Lps 123057 #1226

wants to merge 6 commits into from

Conversation

mtambara
Copy link

Made some changes, let me know if this still fits your requirements

@liferay-continuous-integration
Copy link
Collaborator

To conserve resources, the PR Tester does not automatically run for every pull.

If your code changes were already tested in another pull, reference that pull in this pull so the test results can be analyzed.

If your pull was never tested, comment "ci:test" to run the PR Tester for this pull.

@drewbrokke
Copy link
Owner

ci:test:sf

@drewbrokke
Copy link
Owner

ci:test:relevant

@drewbrokke
Copy link
Owner

@jeyvison when you have a chance, please test these changes to see if they work for you. If you approve and CI passes, I can forward.

@liferay-continuous-integration
Copy link
Collaborator

✔️ ci:test:sf - 1 out of 1 jobs passed in 11 minutes

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: 8b082086f37d045e275f2667a67724dcf4a597b9

Sender Branch:

Branch Name: LPS-123057
Branch GIT ID: 8334c8dcd95d5ed8a4a6de32091e15dea107818f

1 out of 1jobs PASSED
1 Successful Jobs:
For more details click here.

@drewbrokke
Copy link
Owner

@mtambara nevermind, that was my fault - I made a typo while trying to trigger SF.

Repository owner deleted a comment from liferay-continuous-integration Nov 13, 2020
@drewbrokke
Copy link
Owner

ci:stop

@drewbrokke
Copy link
Owner

@mtambara one quick change requested, see my inline comment. I'll leave the pull open, so you can just force push an update.

@liferay-continuous-integration
Copy link
Collaborator

❌ ci:test:relevant - 0 out of 1 jobs passed in 1 hour 6 minutes

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: 06e707df4454e36051558b8c5deaf7600497f9ab

Copied in Private Modules Branch:

Branch Name: master-private
Branch GIT ID: 8d22a791300569a6c92e7b9152e6ae1c6bdc863a

ci:test:relevant - 0 out of 1 jobs PASSED
For more details click here.

Failures unique to this pull:

  1. test-portal-acceptance-pullrequest(master)
    Job Results:

    0 Jobs Passed.
    1 Job Failed.

    Build was aborted

For upstream results, click here.

@liferay-continuous-integration
Copy link
Collaborator

@liferay-continuous-integration
Copy link
Collaborator

@liferay-continuous-integration
Copy link
Collaborator

Jenkins Build:test-portal-source-format#4871
Jenkins Report:jenkins-report.html
Jenkins Suite: sf
Pull Request:drewbrokke#1226
Spira Release:/Liferay DXP 7.3/Pull Request/ci:test:sf
Spira Release Build:master - mtambara > drewbrokke - PR#1226 - 2020-11-13[06:18:36]
Spira Jenkins Build:publish-spira-report#8050

@jeyvison
Copy link

Just started reviewing :)

:octocat: Sent from GH.

@mtambara
Copy link
Author

ci:test:sf

@mtambara
Copy link
Author

ci:test:relevant

@jeyvison
Copy link

Just started reviewing :)

:octocat: Sent from GH.

@liferay-continuous-integration
Copy link
Collaborator

✔️ ci:test:sf - 1 out of 1 jobs passed in 3 minutes

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: 06e707df4454e36051558b8c5deaf7600497f9ab

Sender Branch:

Branch Name: LPS-123057
Branch GIT ID: 243cdac28f51badb7100fa82ac7ee291d4b1b782

1 out of 1jobs PASSED
1 Successful Jobs:
For more details click here.

@liferay-continuous-integration
Copy link
Collaborator

Jenkins Build:test-portal-source-format#4788
Jenkins Report:jenkins-report.html
Jenkins Suite: sf
Pull Request:drewbrokke#1226
Spira Release:/Liferay DXP 7.3/Pull Request/ci:test:sf
Spira Release Build:master - mtambara > drewbrokke - PR#1226 - 2020-11-13[09:31:45]
Spira Jenkins Build:publish-spira-report#11145

@jeyvison
Copy link

Hi @mtambara , Thanks a lot for you changes, the code is a lot more cleaner now :)

Two notes:

  • You changed back the line pattern validation so LIFERAY_ variables are not a match anymore and therefore are ignored.
  • The problem with removing the _propertyValuePattern validations is that if, for example, i define an Env variable like this:
    LIFERAY_MY_ENV=!@#4XD this property will probably not work and we will never know why since the retrieved value from System.getEnv is not validated anymore and there is no output for letting the developer know, as we have for line pattern matching.

Does it make sense?

@mtambara
Copy link
Author

Hi @jeyvison:
For your first point, LIFERAY_ variables are not being ignored, as seen in testLoadEnvVariable. They'll only be ignored if they're part of the key, but that can't be substituted so there isn't a reason to put the liferay prefix there.

For your second point, config values are allowed to be nearly any string as long as they are inside quotes, parentheses, or brackets. With my change, the substitution will still need to be inside ${} for consistency, but that would already need to be inside quotes, parentheses or brackets and would be caught by the line pattern check if it wasn't.

For example, to substitute for an env var like LIFERAY_MY_ENV=!@#4XD, you would need to have a config line like testKey = "${LIFERAY_MY_ENV}" which would get turned into testKey = "!@#4XD"

@jeyvison
Copy link

@mtambara Man, I completely misunderstood that! Now i got it! I just tested it here and it works perfectly :) Thanks a lot, man :)
@drewbrokke LGTM!

@liferay-continuous-integration
Copy link
Collaborator

✔️ ci:test:stable - 9 out of 9 jobs passed

✔️ ci:test:relevant - 22 out of 22 jobs passed in 1 hour 53 minutes

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: e066954b019e5fcf42ca45b69fd4da595ad58029

Copied in Private Modules Branch:

Branch Name: master-private
Branch GIT ID: 8d22a791300569a6c92e7b9152e6ae1c6bdc863a

ci:test:stable - 9 out of 9 jobs PASSED
9 Successful Jobs:
ci:test:relevant - 22 out of 22 jobs PASSED
22 Successful Jobs:
For more details click here.

@liferay-continuous-integration
Copy link
Collaborator

@drewbrokke
Copy link
Owner

ci:forward

@liferay-continuous-integration
Copy link
Collaborator

CI is automatically triggering the following test suites:

  •     ci:test:relevant
  •     ci:test:sf

The pull request will automatically be forwarded to the user brianchandotcom if the following test suites pass:

  •     ci:test:relevant
  •     ci:test:sf
  •     ci:test:stable

@liferay-continuous-integration
Copy link
Collaborator

Skipping previously passed test suites:
ci:test:relevant
ci:test:sf

@drewbrokke
Copy link
Owner

FYI @dejuknow

@liferay-continuous-integration
Copy link
Collaborator

All required test suite(s) passed.
Forwarding pullrequest to brianchandotcom.

@liferay-continuous-integration
Copy link
Collaborator

Pull request has been successfully forwarded to brianchandotcom#95938

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

Successfully merging this pull request may close these issues.

None yet

5 participants