Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix/complete-natspec-issue-17 #322

Closed
wants to merge 8 commits into from
Closed

Conversation

Jean-Grimal
Copy link
Contributor

@MerlinEgalite
Copy link
Contributor

I think @Rubilmax you must update some of your PRs to point to this one right?

@Rubilmax
Copy link
Collaborator

Why did you change the branch? Couldn't we continue working on the other?

@MerlinEgalite
Copy link
Contributor

Why did you change the branch? Couldn't we continue working on the other?

The other one was polluted with many commits (idk really know why). I suggested a rebase but @Jean-Grimal opted for this solution which works also.

@Rubilmax
Copy link
Collaborator

Why did you change the branch? Couldn't we continue working on the other?

The other one was polluted with many commits (idk really know why). I suggested a rebase but @Jean-Grimal opted for this solution which works also.

Indeed it also works, but it nullifies the work I did to base my work on the previous branch, so I can't support this
The former branch doesn't contain "dead" commit or any weird-looking commit, so I don't get the rationale either

I'll merge this branch into the previous branch to see how it looks and if it looks fine I prefer to still work on the previous branch, which avoids to duplicate my work

@Rubilmax
Copy link
Collaborator

See #302: what's the issue?

If there's none, pls close this PR and move back to the previous branch which I just updated with this work

@Jean-Grimal
Copy link
Contributor Author

See #302: what's the issue?

If there's none, pls close this PR and move back to the previous branch which I just updated with this work

It looks fine. Actually I just didn't have enough git skills, that's why I created a new branch.
I didn't know that you based you work on the previous branch sorry.

@MerlinEgalite
Copy link
Contributor

See #302: what's the issue?

If there's none, pls close this PR and move back to the previous branch which I just updated with this work

The issue was before, perhaps a missing merge?

see below

Screenshot 2023-10-24 at 12 29 07

@Rubilmax
Copy link
Collaborator

Alright, then it's all sorted out!

@Rubilmax Rubilmax closed this Oct 24, 2023
@MerlinEgalite MerlinEgalite deleted the fix/issue-17-clean branch November 9, 2023 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants