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.

Sign up for GitHub

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

Jump to bottom

Manually order config codecs #1106

Merged
merged 4 commits into from
Oct 23, 2023
Merged

Manually order config codecs #1106

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
861abb5
Update codec ordering
thomashoneyman Oct 23, 2023
704baee
execArgs -> exec_args
thomashoneyman Oct 23, 2023
5917a75
Swap order of test config creation in Config.js
thomashoneyman Oct 23, 2023
df06e17
reduce use of exec_args
thomashoneyman Oct 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
142 changes: 88 additions & 54 deletions README.md
View file
Open in desktop

Large diffs are not rendered by default.

1 change: 0 additions & 1 deletion bin/src/Flags.purs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,4 +285,3 @@ depsOnly =
( O.long "deps-only"
<> O.help "Build depedencies only"
)

4 changes: 2 additions & 2 deletions bin/src/Main.purs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -740,7 +740,7 @@ mkRunEnv runArgs { dependencies, purs } = do
runConf f = selected.package.run >>= f

moduleName = fromMaybe "Main" (runArgs.main <|> runConf _.main)
execArgs = fromMaybe [] (runArgs.execArgs <|> runConf _.execArgs)
execArgs = fromMaybe [] (runArgs.execArgs <|> runConf _.exec_args)

runOptions =
{ moduleName
Expand Down Expand Up @@ -770,7 +770,7 @@ mkTestEnv testArgs { dependencies, purs } = do
testConf f = selected.package.test >>= f

moduleName = fromMaybe "Test.Main" (testConf (_.main >>> Just))
execArgs = fromMaybe [] (testArgs.execArgs <|> testConf _.execArgs)
execArgs = fromMaybe [] (testArgs.execArgs <|> testConf _.exec_args)
in
{ moduleName
, execArgs
Expand Down
152 changes: 76 additions & 76 deletions core/src/Config.purs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ module Spago.Core.Config

import Spago.Core.Prelude

import Data.Array.NonEmpty (NonEmptyArray)
import Data.Array.NonEmpty as NonEmptyArray
import Data.Codec.Argonaut as CA
import Data.Codec.Argonaut.Record as CAR
Expand All @@ -57,17 +56,18 @@ import Registry.Range as Range
import Registry.Sha256 as Sha256
import Registry.Version as Version
import Spago.FS as FS
import Type.Proxy (Proxy(..))

type Config =
{ package :: Maybe PackageConfig
, workspace :: Maybe WorkspaceConfig
}

configCodec :: JsonCodec Config
configCodec = CAR.object "Config"
{ package: CAR.optional packageConfigCodec
, workspace: CAR.optional workspaceConfigCodec
}
configCodec = CA.object "Config"
$ CA.recordPropOptional (Proxy :: _ "package") packageConfigCodec
$ CA.recordPropOptional (Proxy :: _ "workspace") workspaceConfigCodec
$ CA.record
Comment on lines +67 to +70
Copy link
Member Author

@thomashoneyman thomashoneyman Oct 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I generally stuck to the order I saw defined in the type synonyms, with the exception of potentially long lists (like extra_packages), which I generally moved to the end.

Let me know if you want anything ordered differently and it's easy enough to change.


type PackageConfig =
{ name :: PackageName
Expand All @@ -81,16 +81,16 @@ type PackageConfig =
}

packageConfigCodec :: JsonCodec PackageConfig
packageConfigCodec = CAR.object "PackageConfig"
{ name: PackageName.codec
, description: CAR.optional CA.string
, dependencies: dependenciesCodec
, build: CAR.optional packageBuildOptionsCodec
, bundle: CAR.optional bundleConfigCodec
, run: CAR.optional runConfigCodec
, test: CAR.optional testConfigCodec
, publish: CAR.optional publishConfigCodec
}
packageConfigCodec = CA.object "PackageConfig"
$ CA.recordProp (Proxy :: _ "name") PackageName.codec
$ CA.recordPropOptional (Proxy :: _ "description") CA.string
$ CA.recordProp (Proxy :: _ "dependencies") dependenciesCodec
$ CA.recordPropOptional (Proxy :: _ "build") packageBuildOptionsCodec
$ CA.recordPropOptional (Proxy :: _ "bundle") bundleConfigCodec
$ CA.recordPropOptional (Proxy :: _ "run") runConfigCodec
$ CA.recordPropOptional (Proxy :: _ "test") testConfigCodec
$ CA.recordPropOptional (Proxy :: _ "publish") publishConfigCodec
$ CA.record

type PublishConfig =
{ version :: Version
Expand All @@ -101,54 +101,54 @@ type PublishConfig =
}

publishConfigCodec :: JsonCodec PublishConfig
publishConfigCodec = CAR.object "PublishConfig"
{ version: Version.codec
, license: License.codec
, location: CAR.optional Location.codec
, include: CAR.optional (CA.array CA.string)
, exclude: CAR.optional (CA.array CA.string)
}
publishConfigCodec = CA.object "PublishConfig"
$ CA.recordProp (Proxy :: _ "version") Version.codec
$ CA.recordProp (Proxy :: _ "license") License.codec
$ CA.recordPropOptional (Proxy :: _ "location") Location.codec
$ CA.recordPropOptional (Proxy :: _ "include") (CA.array CA.string)
$ CA.recordPropOptional (Proxy :: _ "exclude") (CA.array CA.string)
$ CA.record

type RunConfig =
{ main :: Maybe String
, execArgs :: Maybe (Array String)
, exec_args :: Maybe (Array String)
thomashoneyman marked this conversation as resolved.
Show resolved Hide resolved
}

runConfigCodec :: JsonCodec RunConfig
runConfigCodec = CAR.object "RunConfig"
{ main: CAR.optional CA.string
, execArgs: CAR.optional (CA.array CA.string)
}
runConfigCodec = CA.object "RunConfig"
$ CA.recordPropOptional (Proxy :: _ "main") CA.string
$ CA.recordPropOptional (Proxy :: _ "exec_args") (CA.array CA.string)
$ CA.record

type TestConfig =
{ main :: String
, execArgs :: Maybe (Array String)
, exec_args :: Maybe (Array String)
, dependencies :: Dependencies
, censor_test_warnings :: Maybe CensorBuildWarnings
, strict :: Maybe Boolean
, pedantic_packages :: Maybe Boolean
}

testConfigCodec :: JsonCodec TestConfig
testConfigCodec = CAR.object "TestConfig"
{ main: CA.string
, execArgs: CAR.optional (CA.array CA.string)
, dependencies: dependenciesCodec
, censor_test_warnings: CAR.optional censorBuildWarningsCodec
, strict: CAR.optional CA.boolean
, pedantic_packages: CAR.optional CA.boolean
}
testConfigCodec = CA.object "TestConfig"
$ CA.recordProp (Proxy :: _ "main") CA.string
$ CA.recordPropOptional (Proxy :: _ "exec_args") (CA.array CA.string)
$ CA.recordPropOptional (Proxy :: _ "censor_test_warnings") censorBuildWarningsCodec
$ CA.recordPropOptional (Proxy :: _ "strict") CA.boolean
$ CA.recordPropOptional (Proxy :: _ "pedantic_packages") CA.boolean
$ CA.recordProp (Proxy :: _ "dependencies") dependenciesCodec
$ CA.record

type BackendConfig =
{ cmd :: String
, args :: Maybe (Array String)
}

backendConfigCodec :: JsonCodec BackendConfig
backendConfigCodec = CAR.object "BackendConfig"
{ cmd: CA.string
, args: CAR.optional (CA.array CA.string)
}
backendConfigCodec = CA.object "BackendConfig"
$ CA.recordProp (Proxy :: _ "cmd") CA.string
$ CA.recordPropOptional (Proxy :: _ "args") (CA.array CA.string)
$ CA.record

type PackageBuildOptionsInput =
{ censor_project_warnings :: Maybe CensorBuildWarnings
Expand All @@ -157,11 +157,11 @@ type PackageBuildOptionsInput =
}

packageBuildOptionsCodec :: JsonCodec PackageBuildOptionsInput
packageBuildOptionsCodec = CAR.object "PackageBuildOptionsInput"
{ censor_project_warnings: CAR.optional censorBuildWarningsCodec
, strict: CAR.optional CA.boolean
, pedantic_packages: CAR.optional CA.boolean
}
packageBuildOptionsCodec = CA.object "PackageBuildOptionsInput"
$ CA.recordPropOptional (Proxy :: _ "censor_project_warnings") censorBuildWarningsCodec
$ CA.recordPropOptional (Proxy :: _ "strict") CA.boolean
$ CA.recordPropOptional (Proxy :: _ "pedantic_packages") CA.boolean
$ CA.record

type BundleConfig =
{ minify :: Maybe Boolean
Expand All @@ -173,14 +173,14 @@ type BundleConfig =
}

bundleConfigCodec :: JsonCodec BundleConfig
bundleConfigCodec = CAR.object "BundleConfig"
{ minify: CAR.optional CA.boolean
, module: CAR.optional CA.string
, outfile: CAR.optional CA.string
, platform: CAR.optional bundlePlatformCodec
, type: CAR.optional bundleTypeCodec
, extra_args: CAR.optional (CA.array CA.string)
}
bundleConfigCodec = CA.object "BundleConfig"
$ CA.recordPropOptional (Proxy :: _ "minify") CA.boolean
$ CA.recordPropOptional (Proxy :: _ "module") CA.string
$ CA.recordPropOptional (Proxy :: _ "outfile") CA.string
$ CA.recordPropOptional (Proxy :: _ "platform") bundlePlatformCodec
$ CA.recordPropOptional (Proxy :: _ "type") bundleTypeCodec
$ CA.recordPropOptional (Proxy :: _ "extra_args") (CA.array CA.string)
$ CA.record

data BundlePlatform = BundleNode | BundleBrowser

Expand Down Expand Up @@ -292,13 +292,13 @@ type WorkspaceConfig =
}

workspaceConfigCodec :: JsonCodec WorkspaceConfig
workspaceConfigCodec = CAR.object "WorkspaceConfig"
{ package_set: CAR.optional setAddressCodec
, extra_packages: CAR.optional (Internal.Codec.packageMap extraPackageCodec)
, backend: CAR.optional backendConfigCodec
, build_opts: CAR.optional buildOptionsCodec
, lock: CAR.optional CA.boolean
}
workspaceConfigCodec = CA.object "WorkspaceConfig"
$ CA.recordPropOptional (Proxy :: _ "lock") CA.boolean
$ CA.recordPropOptional (Proxy :: _ "package_set") setAddressCodec
$ CA.recordPropOptional (Proxy :: _ "backend") backendConfigCodec
$ CA.recordPropOptional (Proxy :: _ "build_opts") buildOptionsCodec
$ CA.recordPropOptional (Proxy :: _ "extra_packages") (Internal.Codec.packageMap extraPackageCodec)
$ CA.record

type WorkspaceBuildOptionsInput =
{ output :: Maybe FilePath
Expand All @@ -307,11 +307,11 @@ type WorkspaceBuildOptionsInput =
}

buildOptionsCodec :: JsonCodec WorkspaceBuildOptionsInput
buildOptionsCodec = CAR.object "WorkspaceBuildOptionsInput"
{ output: CAR.optional CA.string
, censor_library_warnings: CAR.optional censorBuildWarningsCodec
, stat_verbosity: CAR.optional statVerbosityCodec
}
buildOptionsCodec = CA.object "WorkspaceBuildOptionsInput"
$ CA.recordPropOptional (Proxy :: _ "output") CA.string
$ CA.recordPropOptional (Proxy :: _ "censor_library_warnings") censorBuildWarningsCodec
$ CA.recordPropOptional (Proxy :: _ "stat_verbosity") statVerbosityCodec
$ CA.record

data CensorBuildWarnings
= CensorAllWarnings
Expand Down Expand Up @@ -457,12 +457,12 @@ type GitPackage =
}

gitPackageCodec :: JsonCodec GitPackage
gitPackageCodec = CAR.object "GitPackage"
{ git: CA.string
, ref: CA.string
, subdir: CAR.optional CA.string
, dependencies: CAR.optional dependenciesCodec
}
gitPackageCodec = CA.object "GitPackage"
$ CA.recordProp (Proxy :: _ "git") CA.string
$ CA.recordProp (Proxy :: _ "ref") CA.string
$ CA.recordPropOptional (Proxy :: _ "subdir") CA.string
$ CA.recordPropOptional (Proxy :: _ "dependencies") dependenciesCodec
$ CA.record

-- | The format of a legacy packages.json package set entry for an individual
-- | package.
Expand All @@ -473,11 +473,11 @@ type LegacyPackageSetEntry =
}

legacyPackageSetEntryCodec :: JsonCodec LegacyPackageSetEntry
legacyPackageSetEntryCodec = CAR.object "LegacyPackageSetEntry"
{ dependencies: CA.array PackageName.codec
, repo: CA.string
, version: CA.string
}
legacyPackageSetEntryCodec = CA.object "LegacyPackageSetEntry"
$ CA.recordProp (Proxy :: _ "repo") CA.string
$ CA.recordProp (Proxy :: _ "version") CA.string
$ CA.recordProp (Proxy :: _ "dependencies") (CA.array PackageName.codec)
$ CA.record

readConfig :: forall a. FilePath -> Spago (LogEnv a) (Either String { doc :: YamlDoc Config, yaml :: Config })
readConfig path = do
Expand Down
2 changes: 1 addition & 1 deletion src/Spago/Command/Init.purs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ defaultConfig' opts =
, run: Nothing
, test: test <#> \{ moduleMain, censorTestWarnings, strict, pedanticPackages, dependencies: testDeps } ->
{ dependencies: fromMaybe (Dependencies Map.empty) testDeps
, execArgs: Nothing
, exec_args: Nothing
, main: moduleMain
, censor_test_warnings: censorTestWarnings
, strict
Expand Down
2 changes: 1 addition & 1 deletion src/Spago/Config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export function addPackagesToConfigImpl(doc, isTest, newPkgs) {

const deps = (() => {
if (isTest) {
const test = getOrElse(pkg, "test", doc.createNode({ dependencies: [], main: "Test.Main" }));
const test = getOrElse(pkg, "test", doc.createNode({ main: "Test.Main", dependencies: [] }));
Copy link
Member Author

Choose a reason for hiding this comment

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

This was the culprit.

return getOrElse(test, "dependencies", doc.createNode([]));
} else {
return getOrElse(pkg, "dependencies", doc.createNode([]))
Expand Down
2 changes: 1 addition & 1 deletion test-fixtures/check-strict.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@ package:
build:
strict: true
workspace:
extra_packages: {}
package_set:
registry: 41.5.0
extra_packages: {}
6 changes: 3 additions & 3 deletions test-fixtures/list-dependencies.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,15 @@
"value": {
"hasTests": true,
"package": {
"name": "aaa2",
"dependencies": [
"console",
"effect",
"prelude"
],
"name": "aaa2",
"test": {
"dependencies": [],
"main": "Subpackage.Test.Main"
"main": "Subpackage.Test.Main",
"dependencies": []
}
},
"path": "subpackage"
Expand Down
12 changes: 6 additions & 6 deletions test-fixtures/list-packages.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,16 @@
"value": {
"hasTests": true,
"package": {
"name": "aaa",
"dependencies": [
"aaa2",
"console",
"effect",
"prelude"
],
"name": "aaa",
"test": {
"dependencies": [],
"main": "Test.Main"
"main": "Test.Main",
"dependencies": []
}
},
"path": "./"
Expand All @@ -24,15 +24,15 @@
"value": {
"hasTests": true,
"package": {
"name": "aaa2",
"dependencies": [
"console",
"effect",
"prelude"
],
"name": "aaa2",
"test": {
"dependencies": [],
"main": "Subpackage.Test.Main"
"main": "Subpackage.Test.Main",
"dependencies": []
}
},
"path": "subpackage"
Expand Down
6 changes: 3 additions & 3 deletions test-fixtures/local-package-set-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
package:
name: aaaa
dependencies:
- console
- effect
- prelude
name: aaaa
test:
dependencies: []
main: Test.Main
dependencies: []
workspace:
extra_packages: {}
package_set:
path: local-package-set.json
extra_packages: {}
6 changes: 3 additions & 3 deletions test-fixtures/local-package-set-config2.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
package:
name: aaaa
dependencies:
- console
- effect
- prelude
name: aaaa
test:
dependencies: []
main: Test.Main
dependencies: []
workspace:
extra_packages: {}
package_set:
path: ../local-package-set.json
extra_packages: {}
Loading
Loading