Skip to content

Conversation

@javadamiri
Copy link

prepare the README file for release-0.1

Copy link
Contributor

@steveblackburn steveblackburn left a comment

Choose a reason for hiding this comment

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

Only minor edits, aside from the use of submodules. I thought this was fixed long ago!

If it is at all possible to fix this, please normalize the jikesrvm repo, getting rid of the use of submodules.

@javadamiri
Copy link
Author

Only minor edits, aside from the use of submodules. I thought this was fixed long ago!

If it is at all possible to fix this, please normalize the jikesrvm repo, getting rid of the use of submodules.

I finished removing submodules and updating CI scripts and README accordingly

@javadamiri javadamiri requested review from steveblackburn and removed request for qinsoon October 30, 2020 10:38
mmtk/Cargo.toml Outdated
lazy_static = "1.1"
log = {version = "0.4", features = ["max_level_trace", "release_max_level_off"] }
mmtk = { path = "../repos/mmtk-core" }
mmtk = { git = "ssh://[email protected]/mmtk/mmtk-core.git" }
Copy link
Member

Choose a reason for hiding this comment

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

You would need a rev to specify the revision. You can check the Cargo.toml for OpenJDK: https://github.com/mmtk/mmtk-openjdk/blob/7dd5ef78f537b197700feec913890e7d415b16dc/mmtk/Cargo.toml#L24

With this change, the CI config needs to be updated as well, as the CI runner cannot pull the repo by ssh. Check the step here on OpenJDK CI (https://github.com/mmtk/mmtk-openjdk/blob/7dd5ef78f537b197700feec913890e7d415b16dc/.github/workflows/ci.yml#L19).

Also in mmtk-core, there are a few places you would need some change. Check the difference between JikesRVM and OpenJDK in mmtk-core CI, and make the JikesRVM steps similar to the OpenJDK's.

Make sure that CI on both mmtk-jikesrvm and mmtk-core works. You also need to let mmtk-core CI know that the mmtk-core PR should use this JikesRVM binding branch. Like the comments here: mmtk/mmtk-core#136 (comment)

Please let me know if any of these confuses you. I will explain in more details.

Copy link
Member

Choose a reason for hiding this comment

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

@javadamiri I can do a separate PR to remove the submodule. If you think that is probably better, you can just revert changes around submodules, and I will fix them in a following PR.

Copy link
Author

Choose a reason for hiding this comment

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

You would need a rev to specify the revision. You can check the Cargo.toml for OpenJDK: https://github.com/mmtk/mmtk-openjdk/blob/7dd5ef78f537b197700feec913890e7d415b16dc/mmtk/Cargo.toml#L24

With this change, the CI config needs to be updated as well, as the CI runner cannot pull the repo by ssh. Check the step here on OpenJDK CI (https://github.com/mmtk/mmtk-openjdk/blob/7dd5ef78f537b197700feec913890e7d415b16dc/.github/workflows/ci.yml#L19).

Also in mmtk-core, there are a few places you would need some change. Check the difference between JikesRVM and OpenJDK in mmtk-core CI, and make the JikesRVM steps similar to the OpenJDK's.

Make sure that CI on both mmtk-jikesrvm and mmtk-core works. You also need to let mmtk-core CI know that the mmtk-core PR should use this JikesRVM binding branch. Like the comments here: mmtk/mmtk-core#136 (comment)

Please let me know if any of these confuses you. I will explain in more details.

I can do it

@javadamiri javadamiri force-pushed the README-0.1 branch 10 times, most recently from fa7ff41 to 64a911c Compare November 2, 2020 02:52
@javadamiri javadamiri merged commit aa34507 into master Nov 2, 2020
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.

4 participants