Skip to content

Add include and exclude to publish config #972

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

Merged

Conversation

i-am-the-slime
Copy link
Contributor

This change adds config keys

  • include for specifying an array of globs matching files that shall be published in addition to the default (anything matching in /src/**.purs)
  • exclude for finally specifying an array of globs to remove from the union of what's matched by the default and include

It's the first step to supporting this in the registry. Once this is merged, we'll add support and validation for this in the registry-dev repo and then finally finish with a PR to spago/app.

See #967 for discussion leading to this change.

@@ -94,13 +94,17 @@ type PublishConfig =
{ version :: Version
, license :: License
, location :: Maybe Location
, include :: Maybe (Array FilePath)
, exclude :: Maybe (Array FilePath)
Copy link
Member

Choose a reason for hiding this comment

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

These are really globs, right? Should we newtype and sanitize this the way it’s done in the registry?

https://github.com/purescript/registry-dev/blob/6c1209c9eaad10269a2e33445f9beb61b7ac0d12/lib/src/Internal/Path.purs#L29

Copy link
Member

Choose a reason for hiding this comment

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

This part of the PR is about specifying things in the config file. I am not sure we'd want to parse/validate things here already, all this stuff is mostly about reading things in.

Even in the registry the sanitizePath function that you link is only used at glob-matching time, inside Registry.Foreign.FastGlob.match'.
We use that same function to match globs here, so if I understand correctly we're already getting the sanitization for free.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh so it's impossible to add sources from a sibling project in the same project? I guess we can always relax that if the need ever arises.

Copy link
Member

Choose a reason for hiding this comment

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

It should be impossible to add sources from a sibling project as far as the registry (publishing) is concerned, though I suppose it’s fine for Spago

Copy link
Member

Choose a reason for hiding this comment

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

I don't think it should be fine for Spago either, the proper mechanism for adding sibling projects is through a package dependency.

@f-f f-f merged commit aa9773e into purescript:master Jul 15, 2023
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.

3 participants