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

Add openj9-systemtest repo options #107

Conversation

Nandini-4120
Copy link

What GitHub issue does this PR apply to?

Resolves #94

What changed and why?

Made changes according to the issue description.

@Nandini-4120
Copy link
Author

Hi
I have already validated the necessary ECA agreement but I am getting the following error
Screenshot 2021-10-11 at 02 27 48

@smlambert
Copy link
Contributor

see #94 (comment)

For the ECA validation check failure above, one must ensure that the email address associated with the Eclipse account is the same as the one associated to the github ID / commit or the validation check will fail.

@smlambert smlambert changed the title Fixes #94 Add openj9-systemtest repo options Oct 12, 2021
@smlambert
Copy link
Contributor

The 3 failing checks are unrelated to this PR (related instead to adoptium/aqa-tests#2949).

action.yml Outdated
openj9-systemtestsRepo:
description: 'Personal openj9-systemtests Repo'
required: false
default: 'openj9-systemtests:master'
Copy link
Contributor

Choose a reason for hiding this comment

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

The default value is set up in aqa-tests repo there is no need to redo it in run-aqa. Similar to issue #105

src/runaqa.ts Outdated
}

function getOpenj9SystemTestsRepo(openj9systemtestsRepo: string) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar issue mentioned in #105.

Also openj9systemtests environment variable is

env.OPENJ9_SYSTEMTEST_REPO
env.OPENJ9_SYSTEMTEST_BRANCH

@sophia-guo
Copy link
Contributor

@Nandini-4120 Looks like your PR has a different whitespace setup, which means the whole .ts files were changed. Could you please update your configuration so the PR only shows your changes even without hiding whitespace changes?

@Nandini-4120
Copy link
Author

I have made the desired changes and have executed the following command for the Co-author: Somya. Kindly have a look at the commands and suggest if Somya has been added as the Co-author
Screenshot 2021-10-15 at 11 26 56

@Nandini-4120
Copy link
Author

Hi
Is the PR created by me and Somya(@somya-15) ready to be merged or are there any further changes to be made by us? Please suggest.

@smlambert
Copy link
Contributor

Hi @Nandini-4120 - can you please resolve the conflicts in your branch (as some files have changed since you created your PR). thanks!

@Nandini-4120
Copy link
Author

Hi
We have updated the conflicts in our branch. Please suggest.

@Nandini-4120
Copy link
Author

Hi
We have updated the conflicts in our branch. Please suggest.
The above conflicts that the branch is unrelated as was suggested by you. So kindly have a look if our code can be merged.
Screenshot 2022-01-07 at 14 16 47

@sophia-guo
Copy link
Contributor

There are still conflicts need to be resolved.

@somya-15
Copy link
Contributor

somya-15 commented Jan 7, 2022

@sophia-guo, we are unable to identify other conflicts other than system checks.
Screenshot 2022-01-08 at 12 28 43 AM

Kindly guide us in resolving the unidentified conflicts that are causing issues in merging our code. Thanking you.

@sophia-guo
Copy link
Contributor

Screen Shot 2022-01-07 at 5 49 15 PM

The conflicts are between your branch and repo's master branch, which means your branch has to be rebased. The system check is the test failures, which is different from the branch conflicts.

@smlambert
Copy link
Contributor

See Contributing.md - step 5 for ways to rebase a branch (or reference a git cheat sheet for ways to resolve conflicts in a branch, https://opensource.com/article/20/4/git-merge-conflict).

@Nandini-4120
Copy link
Author

Hi
We have covered all steps regarding rebasing in the following commit :524226bf7c15532e5f485c72e4f9b7426704cbc1 Please suggest.

@smlambert
Copy link
Contributor

I will close this PR as stale and it now fails the ECA check because it picks up commits from sophia's old IBM email address which is no longer active. It also appears to contain merge conflict change lines.

@smlambert smlambert closed this Feb 28, 2022
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.

Add option to pass in openj9 systemtest repo and branch
4 participants