You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "robertlipe (via GitHub)" <gi...@apache.org> on 2023/01/28 16:45:38 UTC
[GitHub] [nuttx] robertlipe opened a new pull request, #8329: Ch32v: a pull request of commits to date
robertlipe opened a new pull request, #8329:
URL: https://github.com/apache/nuttx/pull/8329
## Summary
## Impact
## Testing
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] btashton commented on pull request #8329: Ch32v: a pull request of commits to date
Posted by "btashton (via GitHub)" <gi...@apache.org>.
btashton commented on PR #8329:
URL: https://github.com/apache/nuttx/pull/8329#issuecomment-1407475960
This would do what you want I think and let you create a PR on your fork.
https://github.com/robertlipe/incubator-nuttx/compare/master...robertlipe:incubator-nuttx:ch32v
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] robertlipe commented on pull request #8329: Ch32v: a pull request of commits to date
Posted by "robertlipe (via GitHub)" <gi...@apache.org>.
robertlipe commented on PR #8329:
URL: https://github.com/apache/nuttx/pull/8329#issuecomment-1407473773
>
>
> The confusion is likely a GitHub thing vs a git thing. By default when you
> push a new branch it provides a link to create a PR in the UI, but by
> default it will set it against the *upstream* and not your fork.
>
Thanx, Brennan. That's almost certainly my downfall. There's some setting
in this repo I need to track and whack.
> If I want to work in my playground fist but want to have a "PR" to view I
> will update the target branch in that UI to be master on *my* repo. This
> does mean creating a new PR from my branch to upstream later (you don't
> need a new branch).
>
That was exactly my goal. I wanted a nice web-viewable insight into the
changes to share for internal review. The difference between a github view
of a PR and a 'git diff' is just orders of magnitude of difference of
comprehensibility.
> You can also choose to just not create the PR via the UI when you push and
> instead just use the diff view (although it will push you to create a PR...)
>
Again. Spot on. (Hey, I'll bet you've done this before! :-) I know this
port is going to be hundreds of individual commits and I was planning to
cultivate them in my fork in my repo and then transplant them into the
official tree for review/integration. That would let me quickly iterate on
the development and then, at the end, go through a "commit to upstream"
phase. This PR was for my own review and included a long set of individual
commits. I think it was that very nag at the top of the screen "recent
activity in FOO branch, would you like to make a PR?" that baited me into
this. I was VIEWING my branch in my repo, so it didn't occur to me at all
that the PR would be sent to a different place. That's a pretty goofy
behaviour, but I know that speaking poorly of git's UI doesn't go over well
in all audiences.
It's not "obviously" in the git config.
remote.origin.url=https://github.com/robertlipe/incubator-nuttx.git
I'll find it...somewhere.
I'll go back to debugging assembly code and quit getting frustrated by the
lure of a pointy clicky button. That's obviously too simple for me.
Sorry for the distraction.
Message ID: ***@***.***>
>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] robertlipe commented on pull request #8329: Ch32v: a pull request of commits to date
Posted by "robertlipe (via GitHub)" <gi...@apache.org>.
robertlipe commented on PR #8329:
URL: https://github.com/apache/nuttx/pull/8329#issuecomment-1407464541
Ack. Sorry. I need to figure out how to get these to go to MY repo and not
the public one. Git can be so frustrating to me. The commits and pushes go
to https://github.com/robertlipe/incubator-nuttx/tree/ch32v but the PRs go
to upstream while I'm clearly far from wanting to upstream it.
Apologies for the distraction, but I'll leave this PR open just for
mindshare in case anyone goes looking for a WCH port and wants to help.
I'll go ahead and fold in your comments because they're just good sense and
will need to be done anyway. Thanx for that.
RJL
On Sat, Jan 28, 2023 at 12:53 PM Alan Carvalho de Assis <
***@***.***> wrote:
> @robertlipe <https://github.com/robertlipe> please squash/organize these
> commits, because these many reverts it is difficult to review. You don't
> need to close this PR, just use git rebase -i HEAD~N as explained here:
> https://nuttx.apache.org/docs/latest/contributing/making-changes.html#how-to-include-the-suggestions-on-your-pull-request
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/nuttx/pull/8329#issuecomment-1407463021>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/ACCSD3644UKDRKDEHXNBFI3WUVTLRANCNFSM6AAAAAAUJXM7FA>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] acassis commented on a diff in pull request #8329: Ch32v: a pull request of commits to date
Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on code in PR #8329:
URL: https://github.com/apache/nuttx/pull/8329#discussion_r1089791185
##########
boards/Kconfig:
##########
@@ -2668,6 +2675,7 @@ config ARCH_BOARD
default "axoloti" if ARCH_BOARD_AXOLOTI
default "bambino-200e" if ARCH_BOARD_BAMBINO_200E
default "bl602evb" if ARCH_BOARD_BL602EVB
+ default "ch32v307evt" if ARCH_BOARD_CH32V307EVT
Review Comment:
@robertlipe please note that board names are sorted here, so please move it to below "c5471evm"
##########
arch/risc-v/include/ch32v/irq.h:
##########
@@ -0,0 +1,209 @@
+/****************************************************************************
+ * arch/risc-v/include/bl602/irq.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+#ifndef __ARCH_RISCV_INCLUDE_BL602_IRQ_H
Review Comment:
Also replace all BL602 with CH32V
##########
arch/risc-v/include/ch32v/irq.h:
##########
@@ -0,0 +1,209 @@
+/****************************************************************************
+ * arch/risc-v/include/bl602/irq.h
Review Comment:
Please fix the name replacing bl602 with ch32v
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] btashton commented on pull request #8329: Ch32v: a pull request of commits to date
Posted by "btashton (via GitHub)" <gi...@apache.org>.
btashton commented on PR #8329:
URL: https://github.com/apache/nuttx/pull/8329#issuecomment-1407465722
> Ack. Sorry. I need to figure out how to get these to go to MY repo and not
> the public one. Git can be so frustrating to me. The commits and pushes go
> to https://github.com/robertlipe/incubator-nuttx/tree/ch32v but the PRs go
> to upstream while I'm clearly far from wanting to upstream it.
>
> Apologies for the distraction, but I'll leave this PR open just for
> mindshare in case anyone goes looking for a WCH port and wants to help.
>
> I'll go ahead and fold in your comments because they're just good sense and
> will need to be done anyway. Thanx for that.
>
> RJL
>
> On Sat, Jan 28, 2023 at 12:53 PM Alan Carvalho de Assis <
> ***@***.***> wrote:
>
> > @robertlipe <https://github.com/robertlipe> please squash/organize these
> > commits, because these many reverts it is difficult to review. You don't
> > need to close this PR, just use git rebase -i HEAD~N as explained here:
> > https://nuttx.apache.org/docs/latest/contributing/making-changes.html#how-to-include-the-suggestions-on-your-pull-request
> >
> > —
> > Reply to this email directly, view it on GitHub
> > <https://github.com/apache/nuttx/pull/8329#issuecomment-1407463021>, or
> > unsubscribe
> > <https://github.com/notifications/unsubscribe-auth/ACCSD3644UKDRKDEHXNBFI3WUVTLRANCNFSM6AAAAAAUJXM7FA>
> > .
> > You are receiving this because you were mentioned.Message ID:
> > ***@***.***>
> >
>
The confusion is likely a GitHub thing vs a git thing. By default when you push a new branch it provides a link to create a PR in the UI, but by default it will set it against the _upstream_ and not your fork.
If I want to work in my playground fist but want to have a "PR" to view I will update the target branch in that UI to be master on _my_ repo. This does mean creating a new PR from my branch to upstream later (you don't need a new branch).
You can also choose to just not create the PR via the UI when you push and instead just use the diff view (although it will push you to create a PR...)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] acassis commented on pull request #8329: Ch32v: a pull request of commits to date
Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on PR #8329:
URL: https://github.com/apache/nuttx/pull/8329#issuecomment-1407463021
@robertlipe please squash/organize these commits, because these many reverts it is difficult to review. You don't need to close this PR, just use git rebase -i HEAD~N as explained here: https://nuttx.apache.org/docs/latest/contributing/making-changes.html#how-to-include-the-suggestions-on-your-pull-request
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org