-
Notifications
You must be signed in to change notification settings - Fork 457
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ready for review.
cabal v2-run --project-file=cabal.project.ci semantic-source:test | ||
cd semantic-source && cabal v2-run --project-file=cabal.project.ci semantic-source:test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're excluding this from the top-level project now, so we cd
in first.
@@ -15,7 +15,6 @@ packages: semantic | |||
semantic-ruby | |||
semantic-rust | |||
semantic-scope-graph | |||
semantic-source |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here, and in cabal.project.ci
, we remove semantic-source
from the top-level project, ensuring that it'll be fetched from hackage. This makes it possible to factor out PRs like this one, at the cost of making it less convenient to change things across both semantic
(or any other local package) and semantic-source
.
For this PR I think that's worth it; for the next one I might end up reinstating this. But I wanted to factor the PRs out to begin with to limit scope and to be sure we're handling versioning appropriately.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
at the cost of making it less convenient to change things across both
To clarify, this means that the process would be to merge the PR that touches the "downstream" package, cut a new release and deploy it to hackage, and then open the PR in the "upstream" package?
(Not meaning to suggest a different approach, just making sure I understand how the process would be impacted.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup. Or more precisely: reintroduce semantic-source
as a local package (i.e. add it back to this list) without committing that change, and then do the hackage dance after the fact.
I think the real moral here is that we probably released semantic-source
to hackage too early. Lesson learned.
forPath :: Path.PartClass.AbsRel ar => Path.File ar -> Language | ||
forPath :: FilePath -> Language | ||
forPath path = | ||
let spurious lang = lang `elem` [ "Hack" -- .php files | ||
, "GCC Machine Description" -- .md files | ||
, "XML" -- .tsx files | ||
] | ||
allResults = Lingo.languageName <$> Lingo.languagesForPath (Path.toString path) | ||
allResults = Lingo.languageName <$> Lingo.languagesForPath path |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the only real code change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🥲
@@ -15,7 +15,6 @@ packages: semantic | |||
semantic-ruby | |||
semantic-rust | |||
semantic-scope-graph | |||
semantic-source |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
at the cost of making it less convenient to change things across both
To clarify, this means that the process would be to merge the PR that touches the "downstream" package, cut a new release and deploy it to hackage, and then open the PR in the "upstream" package?
(Not meaning to suggest a different approach, just making sure I understand how the process would be impacted.)
forPath :: Path.PartClass.AbsRel ar => Path.File ar -> Language | ||
forPath :: FilePath -> Language | ||
forPath path = | ||
let spurious lang = lang `elem` [ "Hack" -- .php files | ||
, "GCC Machine Description" -- .md files | ||
, "XML" -- .tsx files | ||
] | ||
allResults = Lingo.languageName <$> Lingo.languagesForPath (Path.toString path) | ||
allResults = Lingo.languageName <$> Lingo.languagesForPath path |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🥲
This PR drops
semantic-source
's dependency onpathtype
, selecting languages fromFilePath
s instead. I'll release the new version once this PR is approved.Our other packages get
semantic-source
fromcabal
, and are constrained to exclude 0.2+, so they shouldn't be disturbed by this change. Updating them to drop thepathtype
dependency is postponed to a later PR.