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

Upgrade to Scalameta 3.2.0 #188

Closed
wants to merge 3 commits into from
Closed

Conversation

xeno-by
Copy link
Member

@xeno-by xeno-by commented Feb 2, 2018

There are some test failures, but I wanted to submit anyway, because I'm not exactly sure how many of those are the consequence of having incompatible Scalafix on the classpath (see scalacenter/scalafix#595 for the Scalafix upgrade pull request).

@xeno-by xeno-by added the tech debt We should have addressed this yesterday label Feb 2, 2018
@xeno-by xeno-by added this to the 0.1.0 milestone Feb 2, 2018
// TODO(olafur) handle local symbols on the fly from a `Document` in go-to-definition
// local symbols don't need to be indexed globally, by skipping them we should
// def isLocalSymbol(sym: String): Boolean =
// !sym.endsWith(".") &&
// !sym.endsWith("#") &&
// !sym.endsWith(")")
// be able to minimize the size of the global index significantly.
// case s.ResolvedName(_, sym, _) if isLocalSymbol(sym) => // Do nothing, local symbol.
case s.ResolvedName(Some(s.Position(start, end)), sym, true) =>
// case s.SymbolOccurrence(_, sym, _) if isLocalSymbol(sym) => // Do nothing, local symbol.
Copy link
Member

Choose a reason for hiding this comment

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

The changes for local symbols will require us to implement this TODO. Local symbols must now either use reference equality or include the original document in their syntax because two local symbols from different source files can have identical structures.

if (ptest(p.COVARIANT.value)) mflip(m.COVARIANT)
if (ptest(p.CONTRAVARIANT.value)) mflip(m.CONTRAVARIANT)
flags
}
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure what is happening here and why this huge if-else chain is necessary 🤔

Copy link
Member

@olafurpg olafurpg left a comment

Choose a reason for hiding this comment

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

We should make the kind/properties => Long conversion reusable.

@laughedelic laughedelic changed the title Upgrade to Scalameta 3.0.0 Upgrade to Scalameta 3.1.0 Feb 5, 2018
@xeno-by xeno-by changed the title Upgrade to Scalameta 3.1.0 Upgrade to Scalameta 3.2.0 Feb 5, 2018
@xeno-by xeno-by closed this Feb 11, 2018
@olafurpg olafurpg mentioned this pull request Apr 4, 2018
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tech debt We should have addressed this yesterday
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants