Skip to content
This repository was archived by the owner on Mar 23, 2021. It is now read-only.

Strip out an directory-based replacements from main pkg's module#78

Merged
myitcv merged 1 commit intomasterfrom
drop_non_version_replace_directives
Jul 29, 2019
Merged

Strip out an directory-based replacements from main pkg's module#78
myitcv merged 1 commit intomasterfrom
drop_non_version_replace_directives

Conversation

@myitcv
Copy link
Copy Markdown
Owner

@myitcv myitcv commented Jul 12, 2019

Per much previous discussion, we do apply replace directives in the main
package's module. But for directory replacements this is not guaranteed
to make sense. Indeed it almost certainly doesn't make sense most of the
time. Therefore, strip out these replace directives, leaving behind the
versioned, non-directory replaces.

@myitcv myitcv force-pushed the drop_non_version_replace_directives branch 4 times, most recently from bdf8333 to 42f704b Compare July 12, 2019 09:35
@myitcv myitcv requested a review from rogpeppe July 12, 2019 09:53
@myitcv myitcv force-pushed the drop_non_version_replace_directives branch from 42f704b to 87fa240 Compare July 12, 2019 14:10
Copy link
Copy Markdown
Collaborator

@rogpeppe rogpeppe left a comment

Choose a reason for hiding this comment

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

LGTM modulo one frivolous and one slightly less frivolous suggestion. Thanks!

Per much previous discussion, we do apply replace directives in the main
package's module. But for directory replacements this is not guaranteed
to make sense. Indeed it almost certainly doesn't make sense most of the
time. Therefore, strip out these replace directives, leaving behind the
versioned, non-directory replaces.
@myitcv myitcv force-pushed the drop_non_version_replace_directives branch from 87fa240 to 4102a1c Compare July 29, 2019 17:28
@myitcv myitcv merged commit 350ab10 into master Jul 29, 2019
@myitcv myitcv deleted the drop_non_version_replace_directives branch July 29, 2019 17:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants