Benchmarks #3
Benchmarks #3
Conversation
|
I think if we do merge it in we should shut off the benchmarking action until we're ready. |
|
@rubinovitz I agree. I'd like to be able to use the checks for CD so we should turn off checks we don't expect to pass |
|
Should I just remove the workflow for now before we merge?
…On Mon, Nov 2 2020 at 10:15 AM, Chris Smothers < ***@***.*** > wrote:
@rubinovitz ( https://github.com/rubinovitz ) I agree. I'd like to be able
to use the checks for CD so we should turn off checks we don't expect to
pass
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub (
#3 (comment)
) , or unsubscribe (
https://github.com/notifications/unsubscribe-auth/AANZMF7NIYGMPWNPDS6A4WDSN3ZMVANCNFSM4TFQG5GA
).
|
shadow-cljs.edn
Outdated
| @@ -18,4 +18,10 @@ | |||
| :output-feature-set :es6} | |||
| :modules {:main {:init-fn example.core/init}} | |||
| :js-options {:resolve {"devcards-marked" {:target :npm :require "marked"} | |||
| "devcards-syntax-highlighter" {:target :npm :require "highlight.js"}}}}}} | |||
| "devcards-syntax-highlighter" {:target :npm :require "highlight.js"}}}} | |||
| :npm-dist {:target :npm-module | |||
There was a problem hiding this comment.
Is this related to this benchmarks. If not please submit it as a separate PR. I know it's a pain, but I'd like to get in the habit of making PRs be about just 1 thing. This makes them easier to review and also makes it more reasonable to enforce squash merges since the final commit message will have a single theme and be revertable with no side effects.
There was a problem hiding this comment.
this is formatting I can remove
package.json
Outdated
| @@ -15,16 +15,30 @@ | |||
| "state", | |||
| "graph" | |||
| ], | |||
| "main": "./dist/homebase.react.js", | |||
There was a problem hiding this comment.
I think this is also part of a separate packaging PR
There was a problem hiding this comment.
I think this is actually a bad conflict resolution from the hacky merge with master I had to do so looking into it
| "dependencies": {} | ||
| "dependencies": {}, | ||
| "bugs": { | ||
| "url": "https://github.com/homebaseio/homebase-react/issues" |
There was a problem hiding this comment.
happy to just kill this
|
@rubinovitz given we're not ready to turn the benchmarks on yet what do you think about?
|
|
Yeah there’s some of this that are resolving conflicts that emerged when a bunch was merged in while this branch was broken. I can go back and go through it
…On Fri, Nov 6 2020 at 7:00 AM, Chris Smothers < ***@***.*** > wrote:
***@***.**** commented on this pull request.
In package.json (
#3 (comment)
) :
> "shadow-cljs": "2.11.4"
},
- "dependencies": {}
+
"dependencies": {},
+ "bugs": {
+ "url":
"https://github.com/homebaseio/homebase-react/issues"
and this
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub (
#3 (review)
) , or unsubscribe (
https://github.com/notifications/unsubscribe-auth/AANZMFYWBYIB2HG73MYNLTTSOQFRZANCNFSM4TFQG5GA
).
|
|
@smothers The packaging code is a result of me having to manually merge a bunch of code and accidentally modifying the packaging code. I'll go back and remove the packaging code changes. |
#1 is just going to occur when I remove the packaging changes the arose during the messy merge, #2 is fine |
|
@rubinovitz reduce number of components to 100 |
- take better advantage of homebase-react v0.1.1 attribute tracking
@smothers here it is, back from the dead. Happy Halloween! 💀
If it looks good: I can merge it in if I'm online or if you want to do the manual merge feel free.