jhass changed the topic of #crystal-lang to: The Crystal programming language | https://crystal-lang.org | Crystal 0.35.1 | Fund Crystal's development: https://crystal-lang.org/sponsors | GH: https://github.com/crystal-lang/crystal | Docs: https://crystal-lang.org/docs | Gitter: https://gitter.im/crystal-lang/crystal
kreyren has quit [Ping timeout: 240 seconds]
daemonwrangler has joined #crystal-lang
<FromGitter> <HertzDevil> ```def foo(block : -> -> Bool -> -> String) ⏎ end``` [https://gitter.im/crystal-lang/crystal?at=5fbdab70ba69631c749b1342]
<FromGitter> <Blacksmoke16> :S lovely
<FromGitter> <HertzDevil> that's a `Proc(Proc(Proc(Bool)), Proc(String))`
<FromGitter> <HertzDevil> and the problem is to determine which parentheses around the `->` are necessary, provided that type
<FromGitter> <HertzDevil> luckily the docs generator doesn't seem to use `*` and probably `[]` too (`Pointer` and `StaticArray` are written out)
<FromGitter> <Blacksmoke16> `Void******` :S
<FromGitter> <Blacksmoke16> whats the reasoning for not just using paran all the time?
<FromGitter> <Blacksmoke16> given they're docs readability should be the higher priority
<FromGitter> <HertzDevil> i would have preferred parentheses at all places too
<FromGitter> <HertzDevil> comma binding closer to `->` than to generic arg list is a mistake
<FromGitter> <HertzDevil> `Tuple(Int32, Int32 ->)` is a 1-tuple of a function taking two arguments, not a 2-tuple of an integer and a function taking one argument
<FromGitter> <HertzDevil> you have to parenthesize just the `Int32 ->` to make it a 2-tuple
<FromGitter> <HertzDevil> otoh `Tuple(-> Int32, Int32)` *is* a 2-tuple
<FromGitter> <HertzDevil> but `Tuple(Int32 -> Int32, Int32)` is a syntax error
<FromGitter> <Blacksmoke16> sounds like a mess
f1reflyylmao has joined #crystal-lang
f1refly has quit [Ping timeout: 256 seconds]
avane has quit [Quit: ZNC - https://znc.in]
avane has joined #crystal-lang
<FromGitter> <Blacksmoke16> heres prob a simple question, how would you wrap a regex expression?
<FromGitter> <Blacksmoke16> like given `/\d+/`, create a regex like `/^\d+$/`
<FromGitter> <Blacksmoke16> `Regex.new "^#{pattern}$"` i guess
<FromGitter> <Blacksmoke16> maybe `to_s`/`inspect` is buggy then, because i wouldn't expect printing it to be like `(?-imsx:^(?-imsx:\d+)$)` :/
<FromGitter> <Blacksmoke16> maybe there should be a `Regex.join(*patterns : String | Regex)`
<oprypin> @Blacksmoke16: :D you wouldn't believe `/^#{pattern}$/`
<oprypin> and that output you quoted is actually correct
f1reflyylmao has quit [Quit: bye fags]
f1refly has joined #crystal-lang
<FromGitter> <HertzDevil> it's just pcre being pcre
<raz> is that still a literal then tho? (the other day i learned there's actually a big performance diff between Regex.new vs /foo/)
<raz> ah, found it: https://github.com/crystal-lang/crystal/blob/1a97e71926143cfed8bcd7acde1c0655213d5059/src/compiler/crystal/semantic/literal_expander.cr#L158 (looks like it's indeed gonna be a bit slower, but might not matter depending on use-case of course)
kreyren has joined #crystal-lang
kreyren has quit [Remote host closed the connection]
kreyren has joined #crystal-lang
<straight-shoota> to avoid performance impact you can simply store the regex instance in a class var, constant or whatever
<straight-shoota> @Blacksmoke16 the output format is explained in https://crystal-lang.org/api/0.35.1/Regex.html#to_s(io:IO):Nil-instance-method
<raz> Blacksmoke16: would love to see RapidAthena with an API like this: https://carc.in/#/r/a033 (i believe the param-validation could actually be done more elegantly with your annotation magic)
<FromGitter> <tenebrousedge> this feels like running the other way from MVC
<raz> could still be mixed with MVC with some tweaks. but yes, the focus is on "rapid", minimum boilerplate
<jhass> MVC is not a holy grail by the way https://www.youtube.com/watch?v=WpkDN78P884
<FromGitter> <tenebrousedge> not a holy grail by any means, but it is a reasonable way to model components, with as much empirical validation as one can reasonably expect
<raz> yea, MVC is blessing & curse in one. the above approach is mostly for cases where MVC would be overkill, but also easily tweakable to cooperate with it (i.e. there's obv no law the url-mapping has to be taken from the class-path - i just found that the easiest for my little purpose here)
<raz> the main selling point (to myself) is that i can now write my API with only 2 lines of boilerplate per endpoint. (ok, 4 if you count the 'end's)
<FromGitter> <Blacksmoke16> oprypin straight-shoota yea the format makes sense. Just wasn't expecting it. I wonder if this is something related to straight-shoota's RFC. I.e. should `to_s` not include the other stuff when you just want to see the pattern
<straight-shoota> yeah maybe
<FromGitter> <Blacksmoke16> `{"code":422,"message":"Parameter 'page' of value '5' violated a constraint: 'Parameter 'page' value does not match requirements: (?-imsx:^(?-imsx:\\d{2})$)'\n"}` just looks bad :/
<FromGitter> <Blacksmoke16> even if its semantically correct
<straight-shoota> I'm planning on revisting all #to_s and #inspect methods in stdlib :D
<jhass> MVC is a design pattern that is notorously misused as an architecture pattern. The talk I linked above makes a very great point about this and is worth every minute of your time :)
<raz> ^ +1 - watching it rn
<FromGitter> <Blacksmoke16> ha how about that :p add this one to your list
<FromGitter> <Blacksmoke16> raz: im not sure what benefit im getting from that example
<FromGitter> <Blacksmoke16> i guess you save some lines but isnt the route path and params kinda cryptic?
<FromGitter> <Blacksmoke16> like are `photo_url` and `description` route params? query? form data?
<raz> Blacksmoke16: yup, true. (atm the param macro just looks at route, query, form in that order but that should be more explicit). i think the main point is the auto-wrapping into json-api and minimization of boilerplate. i'm not keen on the cryptic class-to-url-thingy. but in my first experiments realized it's actually great for testing (no guessing what a controller may be called or what methods it has)
<FromGitter> <Blacksmoke16> im actually in the process of enhancing how query/form data params to used in athena
<raz> yay. yep, that keeps looking closer and closer to what i want :)
<raz> (my api is admittedly a bit extreme, but well, i'll do what's needed to drag these other frameworks in my direction :p)
<raz> and your param handling with the annotations is def *much* cleaner
* raz jelly
<FromGitter> <Blacksmoke16> :slight_smile:
<FromGitter> <Blacksmoke16> yea..currently the `@[ART::QueryParam]` doesnt actually do anything 🙈 ha
<FromGitter> <Blacksmoke16> and fwiw ⏎ ⏎ > i think the main point is the auto-wrapping into json-api ⏎ ⏎ Thats the default Athena behavior too 😉 [https://gitter.im/crystal-lang/crystal?at=5fbe7615abf6a739a6b4ed09]
<FromGitter> <Blacksmoke16> er `json-api` is a specific format isnt it
<raz> but plain json wrapping is also a good start already
<FromGitter> <Blacksmoke16> yea by default non `ART::Response` objects are JSON serialized
<raz> esp. when exceptions bubble up (that part most frameworks painfully neglect and barf something useless to the client)
<FromGitter> <Blacksmoke16> i have a dedicated exception class to handle that. if the exception is a child of that class, the code/message is set to the related http code of the error. Otherwise its a 500
<FromGitter> <Blacksmoke16> which would indicate some error context was missed
<raz> yea but is the http 500 also properly wrapped? most frameworks have an annoyingly inconsistent collection of error handlers (400 vs 500) one needs to find and override to make sure _all_ responses are well-formed. and then, more often than not, half of the handlers don't have access to the context for a meaningful response (and logging)
<raz> just for rails alone i have my own little notebook with all the tweaks needed to have proper errors and logging in rails3, 4, 5...
<FromGitter> <Blacksmoke16> `{"code":500,"message":"Internal Server Error"}`
<raz> blacksmoke16: yup but in ENV=development/staging i want the backtrace with that, in prod i don't. also in all env's i want it properly logged with request-id and bt. does have athena good defaults in these areas? :)
<FromGitter> <Blacksmoke16> not by default no, but would be pretty trivial to add
<raz> def worth adding. it's a tragedy how few frameworks get these basics right, even the ones that claim to be geared towards apis. everybody only ever cares about the happy-path :(
_ht has joined #crystal-lang
<FromGitter> <Blacksmoke16> backtraces would prob just be a exception listener that could do something, either handle the exception differently, or add some Fiber context
<raz> well, for me as a would-be user, it would be something i have no clue about and need to figure out
<FromGitter> <Blacksmoke16> deff sounds like something i could add to the wiki
kreyren has quit [Ping timeout: 240 seconds]
<FromGitter> <j8r> not a good idea to use GitHub wiki :/
<FromGitter> <Blacksmoke16> no?
<FromGitter> <j8r> in general
<raz> yea, the default should be good or at least be clearly documented. otherwise, over time your framework ends up like all the others. with an entourage of dusty SO and blog posts (https://stackoverflow.com/questions/51255573/rails-5-api-app-with-json-error-response-instead-of-html-error-response), all desperately trying to somehow get it right. nobody ever being sure what the proper way really is.
<FromGitter> <j8r> @Blacksmoke16 because we can't clone, submit PRs, make it a site
<raz> yeh, github wiki is an UX-disaster (but still way better than nothing)
<FromGitter> <j8r> apparently we can clone https://github.com/athena-framework/athena.wiki.git
<FromGitter> <j8r> sure
<FromGitter> <j8r> for a brand new wiki, better starting with a new repo with markdowns
<FromGitter> <j8r> and GitHub pages can be used out-of-the box
<raz> yup, also https://www.gitbook.com/ is great (free for OSS)
<raz> just uses a normal repo under the hood
<raz> adds pretty rendering, search etc.
<FromGitter> <Blacksmoke16> fair point
<raz> (or one of the bazillion doc generators, mkdocs et al - but they're usually a bit more effort to setup properly)
kreyren has joined #crystal-lang
<FromGitter> <j8r> raz: for a related info, Crystal is moving from GitBook to mkdocs
<raz> ah yea, can make sense (better customizability)
<raz> just given blacksmoke's documentation team is prob rather small, gitbook might be easier to get started with :D
<FromGitter> <Blacksmoke16> just a bit :p
<FromGitter> <Blacksmoke16> in regards to the request id thing. maybe just not including it by default like cors listener would be the way to go
<oprypin> raz: ew wtf gitbook. it's literally not even easier. and i don't have to get into the lock-in part
<raz> oprypin: hm, there is lock-in? can't say i have used them extensively, but isn't it just a repo with a bunch of md files
<oprypin> Blacksmoke16: or others - please loop me in if you plan to make textual docs. I'll push mkdocs of course
<FromGitter> <Blacksmoke16> probably will want to at some point
Nekka has quit [Ping timeout: 246 seconds]
Nekka has joined #crystal-lang
kreyren has quit [Remote host closed the connection]
kreyren has joined #crystal-lang
kreyren has quit [Remote host closed the connection]
kreyren has joined #crystal-lang
kreyren has quit [Remote host closed the connection]
kreyren has joined #crystal-lang
kreyren has quit [Remote host closed the connection]
cloaked1 has joined #crystal-lang
<oprypin> well at least it looks like it'll be an easy bug report https://bpa.st/MJZA
kreyren has joined #crystal-lang
kreyren has quit [Remote host closed the connection]
<oprypin> nope. is not easy
<oprypin> i somehow got a hold of an URI object that crashes when it's being printed
<oprypin> oh no it's a string
<straight-shoota> wow
<straight-shoota> more wow
cloaked1 has quit [Quit: http://quassel-irc.org - Chat comfortably. Anywhere.]
<FromGitter> <Blacksmoke16> Lovely
<FromGitter> <Blacksmoke16> Much wow
<oprypin> ooooh i think it's a proc capture bug yet again
<oprypin> because i went all the way back to my own code with nobody modifying the url in between, but the url somehow is a string at address 0x4
<oprypin> not fixed on master
<FromGitter> <tenebrousedge> D:
<FromGitter> <tenebrousedge> some of the proc bugs got fixed
DTZUZU has quit [Read error: Connection reset by peer]
<oprypin> the kicker is that halite (very unfortunately) captures the passed block
<FromGitter> <Blacksmoke16> im assuming it works with the stdlib client/
<FromGitter> <Blacksmoke16> ?
<oprypin> Blacksmoke16, yea but thats not the point
<oprypin> it's not that halite is broken, it's just innocently capturing this block and i think `url` gets corrupted
kreyren has joined #crystal-lang
DTZUZU has joined #crystal-lang
<oprypin> at this point it's quite certain that crystal tries to use a `String | Nil` currently being `nil` as if it was just a `String`
kreyren has quit [Remote host closed the connection]
<jhass> could use a second pair of 👀 Should this update @out_count before the return? https://github.com/crystal-lang/crystal/blob/master/src/io/buffered.cr#L150-L152
kreyren has joined #crystal-lang
<jhass> uh, wait .flush zeros out_count
<jhass> right?
<oprypin> oh no
<oprypin> just try and see! debug print that line
<jhass> I mean I arrived here because camo.cr would return the wrong content-length header for some things
<jhass> :D
<jhass> yeah I think out_count is just an internal to IO::Buffered meant as an index into the current buffer
<jhass> judging from lines 155 and 159
<jhass> well and 219 especially
<straight-shoota> yes, @out_count is bound to @out_buffer
<straight-shoota> seems http response missuses that
<jhass> so... do I need to find a failing spec for this or will you accept a PR just switching it to some internal tracking thing? :D
<straight-shoota> we should have specs for this
<straight-shoota> not having specs causes such issues in the first place
<jhass> oh wait
<straight-shoota> a spec that writes a response bigger than buffer size should present the error
<jhass> maybe it's not actually an issue
<jhass> This only can get set when the writes until .close are < buffer size
<jhass> because if they're >= buffer size then there's a flush which causes the headers to get written
<jhass> so Ary's commit is enough to fix the issue I'm seeing
<straight-shoota> right
<straight-shoota> it's still feels a bit flaky
<straight-shoota> but probably okay
<jhass> ack
<oprypin> Blacksmoke16,
<FromGitter> <Blacksmoke16> oh snap
<FromGitter> <j8r> on Windows?
<oprypin> j8r, what no
<FromGitter> <j8r> I don't have any issue
<FromGitter> <j8r> nvm, was slow to compile, I thought it was just looping
<FromGitter> <j8r> can be shortened with `HTTP::Client.new(uri).exec "GET", "/", &block`
<FromGitter> <ASPgaming> hello
<FromGitter> <j8r> `HTTP::Client.new(nil)` compiles... ?!
<FromGitter> <j8r> my fault,
<FromGitter> <j8r> I forgot that invalid code that is never executed is fine
kreyren has quit [Remote host closed the connection]
<FromGitter> <Blacksmoke16> o/
<FromGitter> <ASPgaming> So im new to Crystal and used ruby briefly. I like writing emulators will this be good for like a CHIP-8 emulator or even an NES emulator?
<FromGitter> <j8r> yep, I think so
<FromGitter> <j8r> Having Go emulators out there, Crystal can't be worse at it
<FromGitter> <ASPgaming> What do you mean? Are you saying the Crystal is good for it or worse than GO?
<FromGitter> <Blacksmoke16> @oprypin ⏎ ⏎ ```code paste, see link``` [https://gitter.im/crystal-lang/crystal?at=5fbeb82629cc4d73482f4d73]
<FromGitter> <Blacksmoke16> `URI.parse` is working with a ptr of the uri
<FromGitter> <j8r> @ASPgaming Crystal can't be worse as making emulators as Go
<FromGitter> <j8r> both being roughly similar for high/low level programming
<FromGitter> <tenebrousedge> ^
<FromGitter> <ASPgaming> Ah Ok. I was taking a look at sdl but i cant seem to understand how to install it?
<FromGitter> <tenebrousedge> Go is more verbose and explicit
<oprypin> Blacksmoke16, wow so is this specific to uri parse!!?
<FromGitter> <ASPgaming> Im on fedora btw.
<FromGitter> <Blacksmoke16> oprypin: yea seems like it
<oprypin> Blacksmoke16, please post on https://github.com/crystal-lang/crystal/issues/9970
yxhuvud has quit [Remote host closed the connection]
<FromGitter> <tenebrousedge> @ASPgaming what did you try?
<FromGitter> <j8r> @ASPgaming dnf install sdl2-devel, something like this?
<FromGitter> <watzon> Crystal is actually somewhat faster than Go in a lot of cases too
<FromGitter> <ASPgaming> Oh Mk I thought i had to be specific on the language i am trying to install sdl2 for
<FromGitter> <watzon> Nice to see out of a much better looking language
yxhuvud has joined #crystal-lang
<FromGitter> <j8r> I was mainly talking on the technical aspect on the lang; both have GC, compiled, Fiber runtime
<FromGitter> <Blacksmoke16> oprypin done
<FromGitter> <j8r> Buth Crystal has 0 cost C bindings
<FromGitter> <ASPgaming> I think i like crystal
<oprypin> Blacksmoke16, hm no i think it's not about that
<oprypin> even if it was about the pointer, the `.strip` creates a new temporary string with its own pointer
kreyren has joined #crystal-lang
<FromGitter> <Blacksmoke16> 👍
kreyren has quit [Remote host closed the connection]
<FromGitter> <Blacksmoke16> adding a `p a` before doing `a = nil` fixes it :S
kreyren has joined #crystal-lang
<FromGitter> <Blacksmoke16> something something, getting unreferenced or something?
<oprypin> the best workaoround i found was writing `a : String? = "foo"`
<oprypin> Blacksmoke16, i think it's just about unions
<FromGitter> <tenebrousedge> I live on a relatively highly traveled local thoroughfare. My persistent memory of the COVID era is going to be the sound of ambulance sirens.
<FromGitter> <Blacksmoke16> oprypin doing like `a = false`
<FromGitter> <Blacksmoke16> `a = 1` ⏎ ⏎ ```code paste, see link``` [https://gitter.im/crystal-lang/crystal?at=5fbebd22477a8b480c19092d]
<FromGitter> <Blacksmoke16> :S
<FromGitter> <Blacksmoke16> got the updates merged, can checkout the docs for em
_ht has quit [Quit: quit!]
<raz> noice!
<jhass> TIL never trust the Content-Length header in Crystal. HTTP::Client automagically inflates gzip bodies, but it can't know the final content length so the Content-Length header stays at the gzipped length
<oprypin> jhass, it 100% should drop content-length in that cas
<jhass> yeah I'm thinking what's the right solution to this
<oprypin> jhass, dont specify any content-length
<straight-shoota> IIRC there was an issue about that
<jhass> 2017 x_x
<straight-shoota> :(
<oprypin> that sucks
<straight-shoota> the backlog of unmerged stuff is just too big
<straight-shoota> there's no way to keep track of close to 200 open PR
<oprypin> i dont understand the comments on that particular PR
<FromGitter> <Blacksmoke16> go from oldest to newest and just start closing them
<jhass> yeah, looking at the diff it seems pretty fine?
<oprypin> seems perfectly mergeable to me, definitely not a downgrade
<straight-shoota> yeah
<jhass> ah the comment is in response to point 2. in the OP
<jhass> let's see, I'll give it a rebase
<oprypin> jhass, ***merge***
<straight-shoota> Blacksmoke16, I don't think blindly closing everything is a good solution because there's often good stuff that shouldn't just vanish in the archives
<jhass> yeah I hate projects with auto-close bots
<FromGitter> <Blacksmoke16> no ofc not. Just meant the stale/no longer relevant ones
<FromGitter> <Blacksmoke16> but the hard part is determining that
<straight-shoota> you need to look at every single one
<oprypin> Blacksmoke16, this particular one would be determined as stale by most definitions
<oprypin> and yet here we are
<FromGitter> <Blacksmoke16> stale yet relevant :p
<FromGitter> <Blacksmoke16> but im sure theres a fair amount that arent useful anymore etc
<straight-shoota> yes, it could've been closed and create an issue instead
<straight-shoota> sifting through is a huge taske
<FromGitter> <Blacksmoke16> yup, would need to look at every PR and the code etc
<straight-shoota> but it could be nice to do as a team effort
<straight-shoota> maybe meet in voice chat for quick comm, looking at the same PRs and find what to do about it
<FromGitter> <Blacksmoke16> that would help. No other way around it
<oprypin> 🥰
<oprypin> 😂
<straight-shoota> could also be worthy of a drinking game :D
<jhass> Just rebasing would have been too easy of course https://p.jhass.eu/97.txt
<FromGitter> <tenebrousedge> :(
<straight-shoota> the StaticFileHandler specs just need decompress: false
<jhass> yeah, fixed that already
<jhass> stumped on the EOFError though
<jhass> ah, found it
<straight-shoota> great
<jhass> alright, let's see what CI thinks https://github.com/crystal-lang/crystal/pull/5212/files
<jhass> fun fact: the cascade that led me to finding this also triggers a crash in Flutter :D
kreyren has quit [Remote host closed the connection]
<jhass> due to this camo.cr would pass on the too short content-length header, which means the downloaded image misses a couple hundred trailing bytes (images don't gzip well) which produces a crash in Flutter when rendering the downloaded image
kreyren has joined #crystal-lang
<oprypin> jhass, how can you justify a force push, deleting their original commit from history, instead of a merge
kreyren has quit [Remote host closed the connection]
<jhass> because 3 years without feedback, no substantial changes to their work and a merge and fix commit would just be noise in the history
<oprypin> jhass, im not sure if you're aware but history in pull request is one thing (merge master into there) and then history in actual master are different things
<jhass> I don't see your problem honestly
<oprypin> i cannot see what this person wrote
<oprypin> (i mean code)
<jhass> why do you need to?
<oprypin> why do you need to delete it?
<oprypin> oh i can tell you why i need
<oprypin> what if the code you pushed is broken but the original one wasnt broken
<jhass> then you ask me to fix it or reset to a7fefa7 or whatever
<jhass> also note how my earlier justification was "no substantial changes to the original"
<oprypin> that has no relevance at all
<jhass> of course if I would replace all the code I'd rather just open my own PR
<oprypin> you dont need your own PR
<oprypin> just push a new merge commit to this one, thats it
<straight-shoota> I agree, merge commit is better
hightower2 has joined #crystal-lang
<straight-shoota> keeps history on line
<straight-shoota> in this case it really doesn't matter that much from UX perspective as in longer PRs, but still
<jhass> I really don't see how history was important in this case
<jhass> and doing dogmatic policies on these things is stupid
<oprypin> jhass, you are insisting on a clearly worse way with 0 reasons *for* it
<jhass> you're just unwilling to accept the reasons I gave as reasons
<oprypin> there have been no reasons
<oprypin> that a merge commit doesn't also satisfy, anyway
<oprypin> and we're comparing the two
<oprypin> there's no way to directly view what exactly you changed. a merge commit is very good for this purpose, however
<jhass> in theory there is, github just unicorns on it
<oprypin> jhass, that compares everything that changed in the repo in 3 years or whatever
<jhass> whatever, you never reviewed the original code so why would you care about the diff
<straight-shoota> I did review the original code
<straight-shoota> ;)
<oprypin> jhass, ok so are you going to go on a case-by-case basis and make a conscious evaluation that there are "no substantial changes" , "you never reviewed the original code" and "don't see how history was important in this case" and only then use the clearly worse way?
<jhass> yes
<straight-shoota> I don't get what the benefit of rebase would be
<oprypin> me neither, i have said that
<jhass> can you even squash merge a merge commit?
<oprypin> yes
<jhass> without having to resolve the conflict again?
<oprypin> yes
<straight-shoota> I used to rebase until I understood the idea of merge commits
<oprypin> the main reason to rebase was that we used to merge PRs without squashing
<jhass> I guess for a single commit PR it doesn't matter
<jhass> I often maintain logical commit histories inside PRs however
<jhass> and would hate to see them squashed
<straight-shoota> I do, too
<jhass> so rebasing is my default
<oprypin> applying a codereview fix is also a logical change
<straight-shoota> Especially with multiple commits rebasing is bad
<jhass> none that needs to land on master
<oprypin> you will not land it on master anyway, everything gets squashed without exception
<jhass> nah, git rebase -i and git rerere make it easy
<jhass> which is a terrible terrible idea
<oprypin> i think the "bad" part was not about difficulty
<jhass> to always squash
<jhass> I hate it really
<straight-shoota> bad = hard for reviewers
<straight-shoota> squash on merge is a different topic
<jhass> true, I often do --fixup commits until a final pre-merge rebase
<jhass> on complex PRs
<oprypin> theres no other reasonable way. the alternative is much worse: it's commits with completely non-working compiler being in history
<straight-shoota> technically when merging into master you could do a rebase clean history
<jhass> if you have a PR history like that, yeah then squash merge is preffered
<straight-shoota> github UI just won't match that to the PR
<jhass> I try to maintain only working commits
<jhass> logical units from one working state to another
<oprypin> straight-shoota, well you could force push and then merge and then it will match but that also sucks
<straight-shoota> yes, I'd prefer to keep the original history in the PR. But that doesn't need to be the same as what get's merged.
<straight-shoota> The only difference is that the PR shows up as closed instead of merged
<straight-shoota> when it was actually merged
<straight-shoota> just not the way github expects it
<oprypin> but i think this is moot, right? squash is mandatory
<straight-shoota> is it?
<oprypin> well in crystal repo
<jhass> we do that at diaspora (not the rebase but merge manually, because we want most stuff to go a different than the default branch). Little script that automates adding the PR issue number and puts the new commit hases into your clipboard to post to the PR. So you have a reference both ways
<straight-shoota> I remember there was a discussion about it... but I don't remember there was a clear decision
<jhass> never has been a source of confusion
<oprypin> straight-shoota, wait then why is every single PR being squashed :D
<jhass> because Manas just decided to force squash merges
<oprypin> well thats what i was saying
<jhass> did the merge have a conflict?
<oprypin> jhass, ^ I thought I could find 1 disadvantage of merging, i thought maybe it would show up in the suggested squash commit message, but it doesnt!
<straight-shoota> Right, I've found the discussion in the core corner
<oprypin> jhass, i re-did with merge conflict and yes, it works
<FromGitter> <j8r> better squash and merge for single commits, really
<FromGitter> <j8r> rebase change the commits hashes
<straight-shoota> ofc
<oprypin> and squash doesn't?
<FromGitter> <j8r> I don't think so?
<straight-shoota> I don't think there's any dispute about single commit merges
<FromGitter> <j8r> merge commit vs rebase, yeah
<straight-shoota> obviously the hash needs to change when the commit is based on a new parent
<FromGitter> <j8r> depends of if we want a flat history, or preserve original commits
<straight-shoota> so the master branch should have linear history setting
<oprypin> jhass, straight-shoota, also this can be useful info https://github.com/crystal-lang/crystal/pull/9904#issuecomment-726428036
<straight-shoota> but I can choose merge commit in the UI
<FromGitter> <j8r> the review history?
<oprypin> j8r, this discussion was almost entirely about review history
<FromGitter> <j8r> The PR review history is no lost
<oprypin> unless you force push. which is what we were discussin
<jhass> I mean even then it's not lost, github preserves the original diffs discussions were started on
<jhass> I really feel like you're inflating the issue a little
<oprypin> im inflating because this is not just about 1 instance but the long-term defaults
<FromGitter> <j8r> The problem is more for bigger PRs
<FromGitter> <j8r> little ones, most of things are easy to track
<straight-shoota> on that we all agree
<oprypin> Gerrit has all this figured out :/
<FromGitter> <j8r> if master is merged in a branch, then it can't be rebase/commit and merged :/
<oprypin> j8r, that is true but also that is a horrible way to merge
* raz grabs some popcorn
<FromGitter> <j8r> rebase and merge or commit and merge?
<oprypin> rebase and merge in github shouldnt be used ever, in my opinion
<oprypin> then not only do you get random non-working commits, but you also cant trace the reason why they are that way
<FromGitter> <j8r> what happens if I merge master in my branch, then use commit and merge?
<oprypin> then history looks a bit ugly
<oprypin> but since we're not doing that kind of merge either, this is moot
<FromGitter> <j8r> even for bigger PRs? Too bad :/
<FromGitter> <j8r> random not-working commits...?
<oprypin> I've been sending what would've been a "big PR" normally but i split things so masterfully
<oprypin> @j8r: yes because people rebase carelessly and then the last commit passes CI and that gets in
<FromGitter> <j8r> with merge too?
<oprypin> j8r, this is about the aproach to the pull request / review history. both with rebase-merge and merge-merge this can be problematic. but with rebase-merge more so. https://github.com/crystal-lang/crystal/pull/3402#issuecomment-254046071 example where crisis was averted. this is from the old days when merge-merge was ok . but i was careless with rebasing within my PR
<straight-shoota> oprypin, and having smaller individual PRs is really awesome in so many ways
<oprypin> what would be really awesome is having dependent PRs
<straight-shoota> yeah
<oprypin> Gerrit has a 1:1 mapping for commit:"PR" but they can be dependent
<FromGitter> <j8r> I see oprypin
<straight-shoota> I think gitlab has that
<oprypin> oh? cool
<FromGitter> <j8r> too bad Git is complex
<FromGitter> <j8r> there should be, like tagging patches to group them
<oprypin> parent issue # is the tagging
<FromGitter> <j8r> and only one way to take patches to another branch :(
<FromGitter> <j8r> oprypin I know, this is a GitHub thing
<oprypin> kinda sucks that mercurial basically died :D
<oprypin> and the world will not find out about obsolete / evolve technique
<straight-shoota> I never used mercurial
<oprypin> basically what's nice is that you can amend a commit but Git just sees it as an entirely new thing, while mercurial also keeps a reference to the old state of this commit
<straight-shoota> oh, bitbucket dropped mercurial earlier this year
<FromGitter> <j8r> I'm looking forward Pijul
<oprypin> thats what i mean..
<FromGitter> <j8r> > applying some change A, and then a set of changes (BC) at once, yields the same result as applying (AB) first, and then C.
<straight-shoota> so that's like (simplified) each commit is bit like a branch?
<oprypin> i guess a little
<oprypin> the next step from there is it can see if you used to have A <- B but now you also have A' (amended), then it makes sense to move B onto A'. `evolve` command does that
<oprypin> in git, if you checked out an older commit and then amended it, well, not much to do there, you're just holding it wrong
<oprypin> i should clarify, `evolve` command doesnt need any arguments
<straight-shoota> o
<straight-shoota> do you have practical experience with mercurial?
<oprypin> mmm yes
<straight-shoota> but then git took over? =)