Skip to content

Add a note on release-for-release matching with core Gramine#101

Closed
dimakuv wants to merge 1 commit intomasterfrom
dimakuv/fix-readmes
Closed

Add a note on release-for-release matching with core Gramine#101
dimakuv wants to merge 1 commit intomasterfrom
dimakuv/fix-readmes

Conversation

@dimakuv
Copy link
Copy Markdown

@dimakuv dimakuv commented May 6, 2024

Additionally, update the notes on OS distros and Gramine versions for GCC and Scikit-learn.


This change is Reviewable

Additionally, update the notes on OS distros and Gramine versions for
GCC and Scikit-learn.

Signed-off-by: Dmitrii Kuvaiskii <dmitrii.kuvaiskii@intel.com>
Copy link
Copy Markdown
Member

@mkow mkow left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @dimakuv)


README.rst line 31 at r1 (raw file):

template, we recommend to remove the comments from your copies as they only add
noise (see e.g. `Memcached
<https://github.com/gramineproject/gramine/tree/master/CI-Examples/memcached>`__

I think this should be a relative URL? Otherwise it will always teleport to master.

Copy link
Copy Markdown
Author

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 1 unresolved discussion, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @mkow)


README.rst line 31 at r1 (raw file):

Previously, mkow (Michał Kowalczyk) wrote…

I think this should be a relative URL? Otherwise it will always teleport to master.

Relative to what? We link from the examples repo to the gramine repo. So if you meant that, I don't think this is allowed in GitHub's RST?

Copy link
Copy Markdown
Member

@mkow mkow left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, all discussions resolved, not enough approvals from maintainers (1 more required)


README.rst line 31 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Relative to what? We link from the examples repo to the gramine repo. So if you meant that, I don't think this is allowed in GitHub's RST?

Ah, I missed that it changes the repo.

Copy link
Copy Markdown
Author

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 1 unresolved discussion, not enough approvals from maintainers (1 more required)

a discussion (no related file):
Blocking comment to first discuss it with all stakeholders (during Tuesday and Wednesday meetings)


Copy link
Copy Markdown
Contributor

@kailun-qin kailun-qin left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion

Copy link
Copy Markdown
Member

@woju woju left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dimakuv)

@dimakuv
Copy link
Copy Markdown
Author

dimakuv commented May 8, 2024

During the Gramine Tuesday meeting, it was decided to keep the current Examples policy as-is, and postpone merging breaking changes such as #99 until after the next Gramine release is done.

@dimakuv dimakuv closed this May 8, 2024
@dimakuv dimakuv deleted the dimakuv/fix-readmes branch May 8, 2024 06:20
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