-
Notifications
You must be signed in to change notification settings - Fork 82
contributions and applying changes in master branch #22
Comments
LGTM. We can always learn and improve with feedback as people get involved. |
I would also propose this: While merging changes to fix builds, one should always include this at start of commit message: This would make it clear for any developer(other than original patch contributor) as to why changes where merged without any review. |
We also need to figure out as to how we should deal with situations similar to recent DesktopRootWindowHost commits. How about this: |
@kalyankondapally PTAL in d7db1fd and feel free to close this issue if appropriated. Thanks. |
LGTM. Closing this one. |
Re-Opening. One issue with pull requests: It could add un-necessary noise to commit log (with un-necessary commits in the branch): Consider the scenario: This process might continue till the final outcome in achieved(which may mean n no of commits). I am bir worried that this would make un-necessary noise in commit log. History/Discussion can always be seen from the closed issues. I am proposing the following after getting a LGTM on current pull request: Commit message can include: |
"I am proposing the following after getting a LGTM on current pull request: I thought we already been doing this, no? |
currently, we would merge the same branch having " having n commits" but what I am proposing is to ask contributor to create a new branch with the changes after he gets LGTM. 1)Set up a pull request. What I am proposing is at Step 3 contributor can set up a separate pull request with the final patch and pointing to the pull request(where discussion happened) in his commit log. Any Owner would be able to merge it. |
If we do agree on this route but think that it's an un-necessary over burden for the contributor(i.e rebasing and publishing a new branch, closing two pull requests.), than the owner doing the merge can take care of this.(Hopefully we are able to automate it some how.) |
ah, I understood you now -- it's all about cleaning up a patch set after comments were made in there and presenting another set instead just pilling up new quick fixes on top of it. I just thought that this was some kind of implicit rule that every experienced owner should follow :) Jokes apart, if you feel like, you can propose an expansion to the: https://github.com/otcshare/ozone-wayland/blob/master/README.md#contributing . I was trying to be concise at first in that section and I think that's how we should be on the whole README.md after all. |
cool, sure I will add and close this issue. |
let's do the review before you close and apply. tkx! |
This is something we are going to try out: One thing we still need to figure out is how we track the history (or relevant discussions) for commits as we would need to close branch after the pull. What I mean is: I upload a branch and all review happens there. Lets say down the line, we want to go back to a commit and understand the rationale behind the change. Now the history is lost as branch is closed. Is there a way we can preserve the history? Thats one reason I proposed earlier (on top) for two pull requests: Either of the approaches are fine for me as long as we are able to preserve history of a commit.(Discussion during review process) |
in the following model, the history of each git commit remains intact with one only merge commit in the top of the tree, presented by the developer. So in the section "users and developers" should: what do you think? |
I moved this section from README to our wiki page as the section was getting too long. I will update the README file accordingly. |
I was looking for some way to share this information on our IRC channel and wiki fits perfectly. I will add the link to our irc channel once we are public |
Closing this now, feel free to fix anything in wiki. |
[17567:17567:0528/100616:ERROR:display.cc(227)] Not implemented reached in virtual void ozonewayland::WaylandDisplay::SetWidgetState(unsigned int, ui::WidgetState, unsigned int, unsigned int) SHOW 1 [17567:17606:0528/100616:WARNING:server_connection_manager.cc(296)] ServerConnectionManager forcing SYNC_AUTH_ERROR [17567:17606:0528/100616:WARNING:syncer_proto_util.cc(277)] Error posting from syncer: Response Code (bogus on error): -1 Content-Length (bogus on error): -1 Server Status: SYNC_AUTH_ERROR [17567:17606:0528/100616:ERROR:get_updates_processor.cc(240)] PostClientToServerMessage() failed during GetUpdates [17567:17584:0528/100617:WARNING:backend_impl.cc(1780)] Destroying invalid entry. [17567:17567:0528/100616:FATAL:input_device.cc(216)] Check failed: input_pointer_. #0 0x7f9f01fd0a99 base::debug::StackTrace::StackTrace() #1 0x7f9f02022a33 logging::LogMessage::~LogMessage() #2 0x7f9f03337610 ozonewayland::WaylandInputDevice::SetCursorType() #3 0x7f9f0332f9c8 ozonewayland::WaylandDisplay::SetWidgetCursor() #4 0x7f9f06f07869 views::DesktopWindowTreeHostWayland::SetCursorNative() #5 0x7f9f033238ad aura::WindowTreeHost::SetCursor() #6 0x7f9f06f1da8d views::DesktopNativeCursorManager::SetCursor() #7 0x7f9f06dcdf5c wm::CursorManager::SetCursor() #8 0x7f9f06dc94d8 wm::CompoundEventFilter::UpdateCursor() #9 0x7f9f06dc99bf wm::CompoundEventFilter::OnMouseEvent() #10 0x7f9f086c0093 ui::EventHandler::OnEvent() #11 0x7f9f086bdcf1 ui::EventDispatcher::DispatchEvent() #12 0x7f9f086bdab7 ui::EventDispatcher::DispatchEventToEventHandlers() #13 0x7f9f086bd79d ui::EventDispatcher::ProcessEvent() #14 0x7f9f086bd476 ui::EventDispatcherDelegate::DispatchEventToTarget() #15 0x7f9f086bd33d ui::EventDispatcherDelegate::DispatchEvent() #16 0x7f9f0331bcbf aura::WindowEventDispatcher::DispatchMouseEnterOrExit() #17 0x7f9f0331dde1 aura::WindowEventDispatcher::PreDispatchMouseEvent() #18 0x7f9f0331c5c1 aura::WindowEventDispatcher::PreDispatchEvent() #19 0x7f9f086bd2dd ui::EventDispatcherDelegate::DispatchEvent() #20 0x7f9f086c15e7 ui::EventProcessor::OnEventFromSource() #21 0x7f9f0331d845 aura::WindowEventDispatcher::SynthesizeMouseMoveEvent() #22 0x7f9f0331fe00 base::internal::RunnableAdapter<>::Run() #23 0x7f9f0331fc87 base::internal::InvokeHelper<>::MakeItSo() #24 0x7f9f0331fab8 base::internal::Invoker<>::Run() #25 0x7f9f00e1beb9 base::Callback<>::Run() #26 0x7f9f02033288 base::MessageLoop::RunTask() #27 0x7f9f020333ac base::MessageLoop::DeferOrRunPendingTask() #28 0x7f9f020338d3 base::MessageLoop::DoWork() #29 0x7f9f01fb45ef base::MessagePumpLibevent::Run() #30 0x7f9f02032dd7 base::MessageLoop::RunHandler() #31 0x7f9f0207300a base::RunLoop::Run() #32 0x7f9f01678366 ChromeBrowserMainParts::MainMessageLoopRun() #33 0x7f9f05851634 content::BrowserMainLoop::RunMainMessageLoopParts() #34 0x7f9f058589dc content::BrowserMainRunnerImpl::Run() #35 0x7f9f0584dba8 content::BrowserMain() #36 0x7f9f01f73f75 content::RunNamedProcessTypeMain() #37 0x7f9f01f74fa1 content::ContentMainRunnerImpl::Run() #38 0x7f9f01f73439 content::ContentMain() #39 0x7f9f00d75344 ChromeMain #40 0x7f9f00d752ea main #41 0x7f9efcbc876d __libc_start_main #42 0x7f9f00d751f9 <unknown>
[17567:17567:0528/100616:ERROR:display.cc(227)] Not implemented reached in virtual void ozonewayland::WaylandDisplay::SetWidgetState(unsigned int, ui::WidgetState, unsigned int, unsigned int) SHOW 1 [17567:17606:0528/100616:WARNING:server_connection_manager.cc(296)] ServerConnectionManager forcing SYNC_AUTH_ERROR [17567:17606:0528/100616:WARNING:syncer_proto_util.cc(277)] Error posting from syncer: Response Code (bogus on error): -1 Content-Length (bogus on error): -1 Server Status: SYNC_AUTH_ERROR [17567:17606:0528/100616:ERROR:get_updates_processor.cc(240)] PostClientToServerMessage() failed during GetUpdates [17567:17584:0528/100617:WARNING:backend_impl.cc(1780)] Destroying invalid entry. [17567:17567:0528/100616:FATAL:input_device.cc(216)] Check failed: input_pointer_. #0 0x7f9f01fd0a99 base::debug::StackTrace::StackTrace() intel#1 0x7f9f02022a33 logging::LogMessage::~LogMessage() intel#2 0x7f9f03337610 ozonewayland::WaylandInputDevice::SetCursorType() intel#3 0x7f9f0332f9c8 ozonewayland::WaylandDisplay::SetWidgetCursor() intel#4 0x7f9f06f07869 views::DesktopWindowTreeHostWayland::SetCursorNative() intel#5 0x7f9f033238ad aura::WindowTreeHost::SetCursor() intel#6 0x7f9f06f1da8d views::DesktopNativeCursorManager::SetCursor() intel#7 0x7f9f06dcdf5c wm::CursorManager::SetCursor() intel#8 0x7f9f06dc94d8 wm::CompoundEventFilter::UpdateCursor() intel#9 0x7f9f06dc99bf wm::CompoundEventFilter::OnMouseEvent() intel#10 0x7f9f086c0093 ui::EventHandler::OnEvent() intel#11 0x7f9f086bdcf1 ui::EventDispatcher::DispatchEvent() intel#12 0x7f9f086bdab7 ui::EventDispatcher::DispatchEventToEventHandlers() intel#13 0x7f9f086bd79d ui::EventDispatcher::ProcessEvent() intel#14 0x7f9f086bd476 ui::EventDispatcherDelegate::DispatchEventToTarget() intel#15 0x7f9f086bd33d ui::EventDispatcherDelegate::DispatchEvent() intel#16 0x7f9f0331bcbf aura::WindowEventDispatcher::DispatchMouseEnterOrExit() intel#17 0x7f9f0331dde1 aura::WindowEventDispatcher::PreDispatchMouseEvent() intel#18 0x7f9f0331c5c1 aura::WindowEventDispatcher::PreDispatchEvent() intel#19 0x7f9f086bd2dd ui::EventDispatcherDelegate::DispatchEvent() intel#20 0x7f9f086c15e7 ui::EventProcessor::OnEventFromSource() intel#21 0x7f9f0331d845 aura::WindowEventDispatcher::SynthesizeMouseMoveEvent() intel#22 0x7f9f0331fe00 base::internal::RunnableAdapter<>::Run() intel#23 0x7f9f0331fc87 base::internal::InvokeHelper<>::MakeItSo() intel#24 0x7f9f0331fab8 base::internal::Invoker<>::Run() intel#25 0x7f9f00e1beb9 base::Callback<>::Run() intel#26 0x7f9f02033288 base::MessageLoop::RunTask() intel#27 0x7f9f020333ac base::MessageLoop::DeferOrRunPendingTask() intel#28 0x7f9f020338d3 base::MessageLoop::DoWork() intel#29 0x7f9f01fb45ef base::MessagePumpLibevent::Run() intel#30 0x7f9f02032dd7 base::MessageLoop::RunHandler() intel#31 0x7f9f0207300a base::RunLoop::Run() intel#32 0x7f9f01678366 ChromeBrowserMainParts::MainMessageLoopRun() intel#33 0x7f9f05851634 content::BrowserMainLoop::RunMainMessageLoopParts() intel#34 0x7f9f058589dc content::BrowserMainRunnerImpl::Run() intel#35 0x7f9f0584dba8 content::BrowserMain() intel#36 0x7f9f01f73f75 content::RunNamedProcessTypeMain() intel#37 0x7f9f01f74fa1 content::ContentMainRunnerImpl::Run() intel#38 0x7f9f01f73439 content::ContentMain() intel#39 0x7f9f00d75344 ChromeMain intel#40 0x7f9f00d752ea main intel#41 0x7f9efcbc876d __libc_start_main intel#42 0x7f9f00d751f9 <unknown> (cherry picked from commit 374e95a)
For the changes in master, I think we need to decide something and then write down so eventual developers can read and contribute easily with the project.
developers need:
owners need:
The text was updated successfully, but these errors were encountered: