Mats Sigge (Jan 22 2022 at 14:48): Mats Sigge (Jan 22 2022 at 14:48):I'm looking around in the repo and at issues, and want to try and get my feet wet by contributing something. I was looking at issue #2130, which seems quite simple to do. But I don't know what the new crate should be named. Is that something which is usually discussed on Zulip first, or should I just comment on the issue? The contributing guidelines say I should talk to an existing contributor on Zulip before starting on an issue. Do I just ask like this, or is there some other stream/topic which is better suited? :smile:
jan kili (Jan 22 2022 at 14:51):This is the place!
Folkert de Vries (Jan 22 2022 at 14:53):@Richard Feldman thoughts?
Folkert de Vries (Jan 22 2022 at 14:54):reporting_macros sounds ok to me, but the point is kinda to remove the macros from reporting
jan kili (Jan 22 2022 at 14:56):Link for convenience: https://github.com/rtfeldman/roc/issues/2130
jan kili (Jan 22 2022 at 14:56):(idk Rust, so this is the extent of my help here, lol)
Mats Sigge (Jan 22 2022 at 14:59):Folkert de Vries said:
reporting_macrossounds ok to me, but the point is kinda to remove the macros from reporting
Maybe error_macros or roc_error_macros if we want to disassociate them from reporting?
Folkert de Vries (Jan 22 2022 at 15:00):yes that sounds fine
Folkert de Vries (Jan 22 2022 at 15:01):I think we usually prefix with roc_
Mats Sigge (Jan 22 2022 at 15:06):I guess I should create a WIP PR? And forgive my Github ignorance, but it says in the contributor guidelines that I should create a branch in the main repo, and that you can't run tests in forks. Do I automatically have commit rights to the main repo and just commit to a branch, or should I fork and code in a branch, and then that branch will be pulled in to the main repo for the PR?
Mats Sigge (Jan 22 2022 at 15:08):Also, thanks for the very quick replies on a Saturday. :smiley: :thank_you:
jan kili (Jan 22 2022 at 15:08):The former
Mats Sigge (Jan 22 2022 at 15:09):Ok, thanks. :+1:
jan kili (Jan 22 2022 at 15:12):I made the fork "mistake" on my first PR :) https://github.com/rtfeldman/roc/pull/2190
Richard Feldman (Jan 22 2022 at 15:48):roc_error_macros sounds good to me
Richard Feldman (Jan 22 2022 at 15:48):and thanks for volunteering to help out @Mats Sigge! :smiley:
Mats Sigge (Jan 23 2022 at 17:34):I think I've fixed issue 2130, so I pushed my changes to a branch and created a PR. However, CI checks failed, and I don't understand why. I can see that cli_run::i386::deriv failed in the +test-rust step, but there is a valgrind error which I can't decipher, and I don't know what deriv actually does, except that it runs for a long time and takes an obscene amount of memory when I run it locally.
The output of the failed CI checks is at https://github.com/rtfeldman/roc/runs/4912191575?check_suite_focus=true
Also, I'm embarrassed to say that I didn't manage to run the full test suite locally, so I ran what I could and then pushed and kept my fingers crossed, since this was just moving code around. I'm on an M1 MacBook Pro, and from what I understand, it's currently not possible to run Valgrind, at least not natively. But I've also tried running earthly and nix-shell, and I get failures with both, even on trunk.
Does anyone understand the CI failure? And is it currently possible to run the full suite locally on an M1 Mac?
Mats Sigge (Jan 23 2022 at 17:35):Oh, and a link to my PR: https://github.com/rtfeldman/roc/pull/2396
Ayaz Hafiz (Jan 23 2022 at 17:36):This was a failure elsewhere that landed on trunk. @Folkert de Vries just merged the fix; a rebase on trunk should resolve the CI failures.
Mats Sigge (Jan 23 2022 at 17:37):Ah, thanks. :smile:
Mats Sigge (Jan 23 2022 at 17:46):And I realize I'm uncertain about rebasing. Do you mean an actual rebase and a force push to the branch, or should I just merge trunk into the branch and push again? Or something else?
jan kili (Jan 23 2022 at 17:47):The former is common
jan kili (Jan 23 2022 at 17:47):I can walk you through the commands if you'd like!
Mats Sigge (Jan 23 2022 at 17:50):Rebase and force push? I think I know how to do it, but please double check me. :smile:. I'm guessing git rebase trunk and then git push --force origin <branch>?
jan kili (Jan 23 2022 at 17:52):Yes! Assuming no conflicts arise
Mats Sigge (Jan 23 2022 at 17:52):Ok, thanks. :smile:
Mats Sigge (Jan 23 2022 at 21:03):Looks like the checks passed this time. :confetti:
So my next question is do I request in Github that someone review my PR, and if so who? Or is it enough to ask here?
jan kili (Jan 23 2022 at 21:08):If I were you, I'd request a review from everyone who wrote authoritatively in the issue. Someone who's available and knowledgeable usually responds within a day or two
jan kili (Jan 23 2022 at 21:10):I think (but don't know) that the "talk to us in Zulip" thing is just for first time contributions, to get to know people and sync on PR patterns. I'm a new contributor, and I do
jan kili (Jan 23 2022 at 21:11):all of my PRs with no Zuliping involved, except for tricky "hey everyone let's discuss" obstacles
jan kili (Jan 23 2022 at 21:12):(and by "request a review" I meant use the GH PR feature in the top right corner)
Mats Sigge (Jan 24 2022 at 11:31):Thanks for all the help. @Richard Feldman approved and merged the issue after I went to bed. :smile:
Richard Feldman (Jan 24 2022 at 12:28):thanks for the contribution! You are the 64th person to make a commit to the Roc code base, a nice round number :smiley:
Last updated: Jul 05 2025 at 12:14 UTC