Skip to content

Conversation

@hasumikin
Copy link

https://github.com/ruby/yamatanooroti/compare/d16c9cce6f99aec87bdc00335d5a8591bc52ba68..d6f94b44134c55a7750db37ccb8692ced9658136

Once this PR is merged, I want to tag v0.0.10 to this repository and write that tag in Reline's Gemfile: https://github.com/hasumikin/reline/blob/reline_face/Gemfile#L9

gem "yamatanooroti", github: "ruby/yamatanooroti", tag: "v0.0.10"

@hasumikin hasumikin requested a review from st0012 October 31, 2023 23:43
@st0012
Copy link
Member

st0012 commented Nov 1, 2023

Since we're the only one using this fork and the latest commit should always be installed, why do we need to tag it?

@hasumikin
Copy link
Author

hasumikin commented Nov 1, 2023

I think that if we clearly record which commit hash of reline corresponds to which commit hash of yamatanooroti, we will avoid unnecessary confusion in case we need to revert reline.
I don't think it is mandatory for this change, but it is something to consider in general. I wonder why we haven't done this before

@st0012
Copy link
Member

st0012 commented Nov 1, 2023

we will avoid unnecessary confusion in case we need to revert reline.

But have we ever done that before? I don't remember there's a time where we need to pin yamatanooroti to a specific version as we can simply patch it for our needs.

Having to tag yamatanooroti in Gemfile means we'll need additional PRs on both IRB and Reline everytime it receives a new commit. It's an extra process for a problem we don't really have at the moment.

@hasumikin
Copy link
Author

everytime it receives a new commit.

I don't mean that.
I'm just proposing similar management as gemspec's add_dependency. It'd not be a super messy thing because we only need to manage the granularity enough to warrant updating the version number, not every commit.

we don't really have at the moment

Yeah, it's true, I also am not willing to spend time discussing this because it's not mandatory for this time as I said before.
Close this PR if you still don't want to merge this.

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.

2 participants