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

Major rearchitecting of aero (and misc bug fixes) #77

Open
wants to merge 28 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@SevereOverfl0w
Copy link
Member

commented May 20, 2019

The major change here is the addition of macros, a new kind of reader
tag. These fall into 3 categories:

  • #or - Instead of evaluating all of it's list of arguments, it only
    evaluates them until it gets a truthy. The same as the clojure macro.

  • #profile, #user, etc. - The case-like macros will only evaluate the
    correct branch as appropriate. These are also implemented using a
    consistent underlying function which means this commit will fix #67.

  • #ref is expressable using this macro system, and is no longer a static
    dependency analysis. The static analysis was fragile. The special-casing
    for it made it harder to implement other #ref-like things such as a
    #relref. This will fix #50.

I've also scanned through the open issues, marked any fixed as closed,
and fixed low-hanging fruit.

This PR is best reviewed commit-by-commit as I've made an effort to explain each change, and make it isolated for easier understanding.

AndreaCrotti added some commits Jan 29, 2019

@SevereOverfl0w SevereOverfl0w force-pushed the v3 branch 5 times, most recently from 71293d6 to aa3045b Jun 1, 2019

SevereOverfl0w added some commits Jun 2, 2019

Make clj/cljs more uniform
* Use tools.reader.edn for cljs case (provides more uniform interface)
* Refer in default-data-readers and *data-readers* to match clojure.core
* Use edn/read and combine with a pushback reader for both clj & cljs (resulting in better
error messages too!)

@SevereOverfl0w SevereOverfl0w force-pushed the v3 branch 3 times, most recently from 892a103 to 262bf78 Jun 2, 2019

SevereOverfl0w added some commits Jun 2, 2019

Fix #envf in cljs with not-found vars
Passing `nil` as the only argument to a javascript function doesn't
allow it to correctly detect the provided number of arguments.  For our
purposes, the return value of `env` should always be a string.
Introduce concept of "macros" to aero
For now, this is an internal refactoring only.  It introduces 3 kinds of
macro:

* `#or` - Instead of evaluating all of it's list of arguments, it only
evaluates them until it gets a truthy. The same as the clojure macro.

* `#profile`, `#user`, etc. - The case-like macros will only evaluate
the correct branch as appropriate.  These are also implemented using a
consistent underlying function which means this commit will fix #67.

* `#ref` is expressable using this macro system, and is no longer a
static dependency analysis.  The static analysis was fragile.  The
special-casing for it made it harder to implement other `#ref`-like
things such as a `#relref`. This will fix #50.
Remove deferred documentation
These will continue to work for the forseeable future, but they do not
serve a purpose any more.

Deferreds were introduced to balance the fact that conditional readers
(like `#or`) evaluate all branches.
Deferreds unfortunately do not compose with other readers.
This results in writing readers which expected a deferred and
automatically unwrap it.

Now that macros are introduced, we can avoid deferreds altogether.
Disambiguate environment var from env
env-vars are gotten with magic parameters, and the env is where resolved
keys live.
Track where expansion failed
This does 2 things:

1. Provides improved error messages when expansion attempts are
exhausted
2. In the case of `#ref`, allows restoring behavior of placing a `nil`
where `#ref` has failed to expand.
Return missing-include from resource-resolver
Missing include behavior should be consistent with the relative-resolver
- which is part of the default resolver.

Fix #65
Add deps.edn
This makes the repo consumable using the clj command line tools, which
in turn makes it easier to consume aero as a git dependency for
forks/patches/pull requests.
Define alpha api for macro tag literals
When moving the multi-methods from aero.core, this has triggered some
bug in self-hosted with how vars are resolved.  In some cases, the
macro-namespace versions are being used instead of the normal namespace,
to workaround this I've introduced some macros from cgrand's macrovich.

These weren't necessary before, so I think it can be removed when
removing these out of alpha.

SevereOverfl0w added some commits Jun 1, 2019

Tighten kv-seq implementation
Some configs tested were tripping up on MapEntry's, which are not
collections in this system, but are in Clojure.  This is an attempt at
tightening that up.
Fix lumo in CI
Without --unsafe-perm, lumo is a script which terminates immediately
with 0 (success!).  The cljs tests haven't actually been running!
Fix tests when $TERM is unset
This test did not reflect the behavior of config.edn
@codecov-io

This comment has been minimized.

Copy link

commented Jun 2, 2019

Codecov Report

❗️ No coverage uploaded for pull request base (master@36b6fa6). Click here to learn what that means.
The diff coverage is 64.31%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master      #77   +/-   ##
=========================================
  Coverage          ?   68.67%           
=========================================
  Files             ?        3           
  Lines             ?      348           
  Branches          ?       16           
=========================================
  Hits              ?      239           
  Misses            ?       93           
  Partials          ?       16
Impacted Files Coverage Δ
src/aero/impl/macro.cljc 100% <100%> (ø)
src/aero/core.cljc 62.6% <53.17%> (ø)
src/aero/alpha/core.cljc 80.58% <80.58%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 36b6fa6...5783fc8. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.