-
ofrnxmrThe main issue is the conflict with contribution guide stating that tob cant merge his own prs
-
ofrnxmrcontributing (probably) the highers # of commits, this would likely be a roadblock
-
ofrnxmrCurrently* the highest* frequency* of commits
-
ofrnxmrAlso, tob also reviews prs which would mean that we essentially lose a reviewer (would need 2 approvals aside from himself)
-
ofrnxmrI'm all for tob having merge pwr, but we should probably maintain the divide between who writes/reviews(approves) the code, who creates the merge list, and who merges it. imo the latter should be uninvolved in the former 2
-
ofrnxmrIn the past, fluffy used to yolo merge a lot of stuff that was "approved" (even stuff with unaddressed review comments or blatant errors). I think the 3 tier system where "devs write and review code -> selsta triages -> luigi merges" is ideal. mixing the tiers sound like a step in the wrong direction imo
-
m-relay<0xfffc:monero.social> luigi1111 I fully support giving tobtoht writer permission.
-
m-relay<0xfffc:monero.social> s/writer/write/
-
Guest20hi
-
Guest20Is there a guide for windows developers to get them on board?
-
Guest20I need to set up the environment.
-
m-relay<jeffro256:monero.social> Good, we need windows developers! I assume you've looked at this: github.com/monero-project/monero?tab=readme-ov-file#on-windows
-
xFFFC0000Guest* ask any question you have.
-
xFFFC0000I see ofrnxmr concern as valid. I personally fully support tobtoht_ to have write access, but that by definition means we are going to lose him on dev/review side.
-
xFFFC0000That is the dilemma
-
tobtoht_i am certainly not going to stop writing or reviewing code, that would be counter-productive
-
tobtoht_i'm fine with sticking to the merge queue for anything that isn't trivial or is outside of my area of expertise
-
tobtoht_i'm okay with a trial period if there is any doubt in my ability to discern what is and isn't a good merge
-
m-relay<rbrunner7:monero.social> I think if we trust them to handle merges properly, we can also trust them to not reviewing some code just to get it merged. And if we insist, more strictly than until now, on *two* reviews before merging, they can't just "march through" with some code anyway.
-
ofrnxmr"Handle merges properly" means sticking to merge queue. His last 2 comments imply that he wants to bypass the queue and directly merge what he feels qualifies. I think this is in good faith, but not a good idea.
-
tobtoht_i will look to selsta to decide what to merge in areas of the codebase i'm not intimately familiar with
-
ofrnxmrthat's precisely the issue. all prs should go through selsta-approved merge list, not only the ones that youre unfamiliar with. And you shouldnt cant merge code that youre involved with the dev or review steps. This leaves you handicapped and doesnt solve the issue if luigi being mia.
-
ofrnxmrAny and all things need to be added to merge queue, they need to be submit / suggested to selsta
-
ofrnxmrThen someone else uninvolved with the selection or development should be merging said list
-
m-relay<rbrunner7:monero.social> Like it's the case so often, clean and simple fundamental positions seem to clash violently with the messy real world. Maybe right and sensible *in theory*, but good luck handling and enforcing it.
-
tobtoht_i don't see why it would make sense to block a maintainer from _maintaining_ parts of the codebase they are most familiar with
-
tobtoht_in my area, i know by know which changes are likely to be controversial and require additional input (beyond 2 approvals)
-
tobtoht_i don't necessarily know who the stakeholders are in parts of the codebase i'm less familiar with, so i can't use the same amount of rigor there and would look to selsta to decide what to merge.
-
m-relay<monero.arbo:matrix.org> I don't have much opinion on the specifics of how to implement/rules but fwiw I basically trust tobtoht_ with my life
-
m-relay<monero.arbo:matrix.org> (slight exaggeration)
-
m-relay<monero.arbo:matrix.org> If we had someone trustworthy who could *just* do merges without losing a reviewer etc I can see how the separation is useful tho
-
ofrnxmrThis isnt abt tob as a person. I trust him too. But as a project, we shouldn't be making such extreme exceptions
-
ofrnxmr"who could *just* do merges" Currently this is luigi
-
m-relay<monero.arbo:matrix.org> well, *another* person since there seems to be agreement that there's currently a bottleneck
-
m-relay<321bob321:monero.social> What about if Luigi appears more often
-
m-relay<monero.arbo:matrix.org> but yes I see your point of course. I'm not familiar with the contribution guidelines will see if I can find them
-
ofrnxmrThats an assumption that is misplaced
-
ofrnxmrmost people fairly assumed that this was about luigi being on the yacht an letting merge queue grow. But the issue that tob wants to solve is that things miss the queue. People making suggestions to the queue, should not be the same people merging them
-
tobtoht_if it's conditional on your approval of the proposal, then i'm ok with doing everything through the merge queue
-
ofrnxmrgithub.com/monero-project/monero/blob/master/docs%2FCONTRIBUTING.md its old but here @lyza
-
m-relay<monero.arbo:matrix.org> yep currently peeping it ty
-
tobtoht_i disagree that maintainers should not merge their own prs. if it's added to the queue, i see no benefit in waiting on luigi for that. merging is taking responsibility, which will promote careful consideration and a degree of conservatism.
-
ofrnxmr1. Need to submit/suggest/defer to selsta regarding anything that should be in queue. 2. Person making submissions, suggestions to merge list, or authoring and reviewing the code, should not be merging the code
-
m-relay<monero.arbo:matrix.org> so if I'm reading correctly merging ones own commits is a no-no but reviewing your own is fine if one is a "core code developer" which seems... not to be defined in this document but is maybe a reference to the so-called "core team"?
-
ofrnxmrno, not core team
-
m-relay<monero.arbo:matrix.org> what is considered a core code developer then
-
ofrnxmrDevelopers working on monero-project/monero
-
m-relay<321bob321:monero.social> FYI the year needs to be updated
-
ofrnxmrPlowsof has a pr for that 😆😆😆😆😆😆 #irony
-
m-relay<monero.arbo:matrix.org> I guess my contribution here is that the contribution guide should be respected and the prospect of changing it should be a separate discussion
-
m-relay<monero.arbo:matrix.org> on the surface this seems logical tho
-
tobtoht_i'm not interested in the role if i'm unable to maintain effectively
-
tobtoht_i'm highly motivated, and i think i can do a good job at making life easier for other developers by promptly merging prs, and making sure development builds work, devs aren't faced with failing ci, and the build system does not fall into ruin.
-
tobtoht_demonstrated by monero-project/monero #9669, monero-project/monero #9631 and four years of experience maintaining feather
-
ofrnxmr💢 irc failed to send my msg and its gone
-
ofrnxmri said.. i think there are 2 things that can be done to increase efficiency without removing the balances (re checks and balances). 2. Someone (devs) helping by submitting items to selsta for consideration in merge queue 2. Regular flushes of queue (like.. weekly) << re the former, there was discussion about dev meetings and a suggestion of mine
-
ofrnxmrwas for them to focus on unaddressed issues, prs needing review, items missing from queue (or from release branch), and assignments. For the he latter, it would be as simple as luigi being available on a regular and reliable basis, barring that, we'd have to find someone who is disconnected from the development side to fill in. This a slippery
-
ofrnxmrslope though, because a lot of people fit that definition
-
tobtoht_what value does a person who is disconnected from the development side add?
-
tobtoht_i honestly don't get this line of reasoning. surely, it can't be their ability to assess whether a change is a net-positive? how the disconnect help in any way?
-
ofrnxmrThat they only ship the package. It avoids value assessments if the person merging the code isnt involved in the selection or creation of which code to merge. Also, the 3 tier system (currently) in place works and would be efficient if not for unreliable merges. I still think dev meetings are a good idea to avoid stuff like this being missed
-
ofrnxmr
-
ofrnxmrThere are 11 prs there that need reviews, and we should have meetings to make sure people volunteer/self assigned to do so
-
tobtoht_it's the slow merges, not the lack of reviews that is the problem
-
tobtoht_i mean meetings would also be beneficial for speeding up reviews, sure
-
tobtoht_if all prs that are added to the merge queue are merged, what benefit is there to waiting longer?
-
tobtoht_and why is selsta uniquely qualified to make executive decisions on what to merge?
-
ofrnxmrthey arent all merged, people test the queue, which occasionally leads to finding issues and problematic prs are removed
-
tobtoht_if the pr wasn't properly tested, then it shouldn't have been to added to the queue
-
ofrnxmrif it has multiple approvals, it was clearly missed by reviewers
-
tobtoht_and what harm is there in reverting if a bad commit does inadvertently get merged?
-
ofrnxmrWe had broken code on master for like 8 months and didnt notice until reports came in after releasing it
-
m-relay<monero.arbo:matrix.org> damn would be nice if it was easier for people to test development builds huh (:
-
tobtoht_the master branch receiving insufficient testing seems like a separate issue
-
tobtoht_bugs will always get caught after a release when it's not just testers running builds
-
tobtoht_issues are more likely to get noticed sooner if development builds weren't broken on rolling release distros
-
ofrnxmrmy point is that, yes, we revert bad commits all the time. And they get added to the queue all the time, due to bad testing or inadequate reviews
-
tobtoht_which they currently are
-
tobtoht_and for which prs are ready and reviewed
-
tobtoht_waiting to be merged
-
ofrnxmrrolling release distros like debian?
-
tobtoht_debian is not a rolling release distro
-
tobtoht_ok, sounds like we should further improve the review process then, not kneecap merges
-
ofrnxmrI didnt say we should kneecap merges. I said we should improve the review process, the queue process, and the merge process
-
ofrnxmrbut luigi wants to enjoy his yacht time 🤷♂️
-
tobtoht_do you think my proposal is a net negative for the project? do the cons outweigh the potential benefits so much so that we must strictly adhere to an ancient contributor document? and will you give me the benefit of the doubt, if only for some predefined trial period?
-
tobtoht_if you think prs could benefit from some time in the queue, why don't we institute 'merge tuesdays' or whatever?
-
m-relay<rbrunner7:monero.social> ... and already time was wasted discussing to exhaustion details that probably will never ever play a real role in anything that would be enough to either merge 10 PRs, review 1 small PR, or write 30 lines of useful C++ code.
-
m-relay<rbrunner7:monero.social> tobtoht_, ofrnxmr won't stop arguing first :)
-
ofrnxmr"details that probably will never ever play a real role in anything" these details are what is is _today_. Id *argue* that these very much play a real role _today_ (not "never ever")
-
ofrnxmrRe merge tuesdays, im on board. But i'd rather have tob (so someone) help with the merge list than do merges. as much as i personally trust tob, I don't think it its a good idea to make exceptions based on trust
-
ofrnxmr.merges
-
xmr-pr9389
-
ofrnxmrmonero-project/monero #9631 there are a few prs here that seem to he adequately reviewed but arent on the merge list. This isn't a luigi issue, its a communication one
-
ofrnxmrThats not to say that there isnt a luigi issue.. just that the swath of prs that are unattended isnt a luigi issue (nor a selsta one). Its a communication issue that could likely be solved with short dev meetings where issues and prs are addresses
-
m-relay<syntheticbird:monero.social> I have the last word
-
m-relay<syntheticbird:monero.social> hehehehehehe
-
ofrnxmrmonero-project/monero #9446 this too needs ack/nacks
-
ofrnxmrIs a precursor to a lot of other work
-
tobtoht_please do leave a comment if you have an opinion
-
luigi1111ofrnxmr> Also, tob also reviews prs which would mean that we essentially lose a reviewer (would need 2 approvals aside from himself) <= tob is a valid reviewer for PRs not made by himself, even if he merges them, IMO
-
luigi1111tobtoht_> if it's conditional on your approval of the proposal, then i'm ok with doing everything through the merge queue <= I think this is the best current workflow. The bot is accessible to everyone I think and history easily viewable being IRC
-
luigi1111even if I still have to merge at least tob's PRs, that's still much very helpful. It takes actual time to go through a merge queue, not just "oh luigi is online, done".
-
ofrnxmrThe major issue being solved here is that tob has important prs that he needs merged in short order. Sure there are other works that it helps with, but the main point is tob's work
-
ofrnxmr"> i don't see why it would make sense to block a maintainer from _maintaining_ parts of the codebase they are most familiar with" i take this to mean that a major part of the goal is to merge his own prs "i'm not interested in the role if i'm unable to maintain effectively"
-
tobtoht_yes, of particular interest are any prs labeled "build system" and "ci", not just my prs. i care about fixing problems that affect other developers, so i'm usually the one fixing them. maintenance work. the role of a maintainer.
-
tobtoht_jeffro's pr's getting blocked because we target an ancient version of macos? i don't like seeing that and it needs fixing.
-
m-relay<syntheticbird:monero.social> Tobtoht_
-
m-relay<syntheticbird:monero.social> CEO
-
m-relay<syntheticbird:monero.social> i failed my matrix formatting i hate my life
-
m-relay<syntheticbird:monero.social> anyway a maintainer is obviously in conflict of interest with itself, since he is a maintainer
-
ofrnxmrmonero-project/monero #9523 right but it has 2 reviews and hasn't been added to queue. This is a communication issue, not a Luigi one
-
m-relay<syntheticbird:monero.social> alright then what about best of both world, toboht_ have write access and we try to organize dev meetings?
-
ofrnxmrwe need someone to chair the meetings
-
ofrnxmrLuigi1111 also, what do you think about "merge tuesdays" ?
-
ofrnxmrAre you available on a scheduled basis to flush the cache
-
tobtoht_ofrnxmr: fwiw, i would ask selsta to sign-off on 9523 first, because i know he cares about platform support. in general, changing runtime requirements should be substantiated and require more approvals.
-
Guest20hi
-
m-relay<ofrnxmr:monero.social> Hi. Still having trouble with your build env?
-
Guest20yea
-
Guest20I wonder what is the recommended way to create a dev env for monero?
-
selstaif another maintainer is added i would prefer if everything still goes through the merge queue
-
tobtoht_alright
-
selstai'm available daily so if something needs to be added it should not cause any delays
-
selstahaving the merge queue for some things and direct merges for others is just going to cause confusion IMO
-
tobtoht_that's fair
-
tobtoht_if i could merge my own pathes that were added to the queue under the condition that they {provide immediate relief to other devs, or have many dependants, or are trivial} that would be great. i'm ok with asking for approval first. for my other stuff i'm ok with waiting on luigi.
-
m-relay<jeffro256:monero.social> Guest20: download the source code, setup msys2 and mingw, install dependencies, boot up IDE of choice, and start tinkering. Are you running into any specific issues?
-
tobtoht_failing ci drives me up a wall, and if have the power to fix it i want to be able to - again, provided it is added to the merge queue
-
Guest20CMake Warning at tests/functional_tests/CMakeLists.txt:79 (message):
-
Guest20functional_tests_rpc and check_missing_rpc_methods skipped, needs the
-
Guest20'requests', 'psutil', 'monotonic', 'zmq', and 'deepdiff' python modules
-
Guest20I installed Python and I also pip installed all these modules. I even added the path to pip...
-
moneromoooMake sure you installed the modules for the version of python which gets used.
-
moneromooo(python 2 and python3 are not so compatible)
-
Guest20the source code seems to have many errors for my compiler, I am using clang, am I supposed to use gcc?
-
moneromoooBoth GCC and CLANG build monero. Very new versions might not though, but that can typically be fixed.
-
moneromoooMake sure to follow directions from the README.
-
Guest20Yea building works, however clangd finds errors that are not really errors. E.g. "bool" not defined lmao
-
Guest20I could include <stdbool.h> of cause... But I think the code should not contain errors in every source code of this kind
-
Guest20then we have this: BYTE_ORDER not defined, it should be defined though
-
Guest20or: variable has incomplete type: "union hash_state"
-
JuliuHi
-
JuliuDon't tell me it's written in pure C
-
Guest20Juliu - you are right, for some reason the compiler wants to treat all files ending with .c or .h as pure C, it does not want to check if the errors are solved by interpreting it as cpp/hpp
-
luigi1111tobtoht_> if i could merge my own pathes that were added to the queue under the condition that they {provide immediate relief to other devs, or have many dependants, or are trivial} that would be great. i'm ok with asking for approval first. for my other stuff i'm ok with waiting on luigi. <= if it has 2 approvals and selsta adds to merge queue that's fine by me. I would also merge my own PRs if I regularly made any
-
JuliuGuest20, if should not matter what the files are named. I was referring to stdbool.h, which isn't needed in C++ since C++ has bool by default
-
Juliu*IT should not ...
-
Guest20If I rename the extention from .c to .cpp or h. to .hpp the errors disappear
-
Guest20Yea it should not matter, I don't know why it does
-
JuliuAsk your compiler
-
ofrnxmr"if it has 2 approvals and selsta adds to merge queue that's fine by me. I would also merge my own PRs if I regularly made any" thats counter to the contrib guidelines
-
selstathough the guidelines were written before we had a merge queue where an external person is in the loop before it gets merged
-
ofrnxmryes but how does it get on the merge queue efficiently, if not by request by the same person who wrote the code
-
Guest20can I clone the dev environment for monero?
-
CTRL_FAILUREGood {greeting_time},
-
CTRL_FAILUREare the algorithms in src/crypto up to date? Or are low level impl from external superior?
-
luigi1111ok. I think I'm going to invite him with the expectation that PRs go through the merge queue as now and non "duh" (like CI etc) PRs from tob won't be merged by him (the simple ones would still need approval obviously). If the contrib guidelines disagree, well whatever we can change them if needed? They should exist to be reasonable
-
luigi1111as for merge tuesdays, I probably can't make every tuesday, but 1-2x per month should be feasible.
-
CTRL_FAILUREe.g.: external/supercop/crypto_sign/ed25519 is probably a better impl
-
ofrnxmrThe contrib guidelines are reasonable. Maintainers should not merge their own patches, except for exceptional circumstances
-
ofrnxmrIts not shall/will not. Its should not, and has exceptions built in. Sounds very reasonable to me. 1x/mth is not enough imo.
-
ofrnxmr"Maintainers SHOULD NOT merge their own patches except in exceptional cases, such as non-responsiveness from other Maintainers for an extended period (more than 30 days) or unless urgent as defined by the Monero Maintainers Team."
-
ofrnxmrCi prs are not always the same thing as build system. Ci is, in some bases, closer to documentation. Documentation has no such restriction
-
ofrnxmr"Maintainers MAY commit changes to non-source documentation directly to the project."
-
luigi1111I think doc changes should still be PRs
-
luigi1111it could be narrowed to "builds failing" or whatever
-
luigi1111(would still need 2 approvals as per the github rules that are now live)
-
ofrnxmrOf course agree that absolutely nothing should be commit w/o a pr, but i dont have the same issue with "own prs" if the content doesnt touch the codesource. (ofc still with approvals)
an hour ago