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

Allow linking to lower level in delta specs. #1598

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

fantasai
Copy link
Contributor

No description provided.

@frivoal
Copy link
Contributor

frivoal commented Jun 3, 2020

Note: the merge conflict indicated by github is a trivially resolved one.

Copy link
Contributor

@nschonni nschonni left a comment

Choose a reason for hiding this comment

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

Copying over the suggestions from the formatter

bikeshed/refs/ReferenceManager.py Outdated Show resolved Hide resolved
bikeshed/refs/ReferenceManager.py Outdated Show resolved Hide resolved
@fantasai fantasai force-pushed the delta-self branch 2 times, most recently from f3d9b55 to 2054fbe Compare March 26, 2021 19:56
@@ -519,6 +523,21 @@ def getRef(
ignoreObsoletes=True,
)

if failure and self.isDelta:
Copy link
Collaborator

@tabatkins tabatkins Jun 18, 2021

Choose a reason for hiding this comment

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

This block doesn't do what you want. latestOnly just finds, for each dfn from each shortname, the highest-level version and then suppresses lower-level versions; if this is a delta spec that doesn't contain a term's dfn, it won't have a dfn in the results anyway so this won't have any effect.

(Also, it would only trigger if the first try completely failed; a confusable reference from another spec would prevent this from activating.)

Copy link
Collaborator

Choose a reason for hiding this comment

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

What you instead want to alter is the ReferenceManager.removeSameSpecRefs() function; that's what unexports all dfns from specs with the same shortname.

(But make sure you still remove refs from the same or later level, so your local copy doesn't get confused with its version in the db, and doesn't link forward to a future version of itself either.)

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