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

feat: webapp commit message support #10

Closed
wants to merge 2 commits into from
Closed

Conversation

0x4007
Copy link

@0x4007 0x4007 commented Apr 11, 2023

GitHub supports suggestions during the pull request review process. This process to make the change to the code, and push the commit is entirely inside of the GitHub pull request review user interface.

The default git message begins with "Update " and so we decided to add support for this on our fork.

More context is available here: ubiquity/ubiquity-dollar#623 (review)

Copy link
Member

@adrians5j adrians5j left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a question. Thanks!

package.json Outdated
"got": "^11.3.0",
"lodash.get": "^4.4.2"
"lodash.get": "^4.4.2",
"typescript": "^5.0.3"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These deps should go into devDeps, no?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Steveantor what do you think

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typescript is required for the build process

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's build-related, this should then be a dev dep.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, needs to be moved to dev-deps in that sense.

@Steveantor
Copy link

@adrians5j how do we resolve the index.js conflict?

@adrians5j
Copy link
Member

Please just resolve the merge conflict. It's just dist/build.

Whatever you do, no worries about it, I'll rebuild again before I release.

@Steveantor
Copy link

Please just resolve the merge conflict. It's just dist/build.

Whatever you do, no worries about it, I'll rebuild again before I release.

I'm not sure how to resolve merge conflicts of the build/transpiled files,
maybe u could use git merge <target_branch> -X theirs

@adrians5j
Copy link
Member

Just accept yours. It's not a problem, because I'll do a rebuild once this is merged.

@0x4007
Copy link
Author

0x4007 commented Apr 19, 2023

I think that's permission to force push @Steveantor

@0x4007
Copy link
Author

0x4007 commented Apr 24, 2023

Steveantor told me that @adrians5j needs to force merge and that from here its out of his hands.

@adrians5j
Copy link
Member

adrians5j commented Apr 26, 2023

I can't do changes in fork branches. Changes can be done only on your side.

P.S. I'm merging via GH, and it prevents me from merging b/c of a conflict.

@0x4007
Copy link
Author

0x4007 commented Apr 26, 2023

I can't do changes in fork branches. Changes can be done only on your side.

You have two options:

  1. Press "." on your keyboard to edit this pull request in the browser.
  2. Or run gh pr checkout 10 locally.

Screenshot 2023-04-26 at 19 35 00

Sorry mate the road ends here with our team.

@Steveantor
Copy link

@adrians5j no more merge conflict

@Steveantor
Copy link

@adrians5j will you look into this PR again?

@0x4007 0x4007 closed this Jul 3, 2023
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