-
Notifications
You must be signed in to change notification settings - Fork 129
fix(task): resolve feedback notification issue #52
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
base: 9.x
Are you sure you want to change the base?
Changes from 4 commits
293b692
69ef185
16373ef
fefd141
508097e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| # Set the default behavior, in case people don't have core.autocrlf set. | ||
| * text=auto eol=lf |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -36,5 +36,4 @@ student-work/ | |
| .idea/ | ||
| .byebug_history | ||
| coverage/ | ||
| .vscode | ||
| _history | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| { | ||
| "files.eol": "\n" | ||
| } |
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a config file - changes to this are typically not something you would commit unless they are part of a necessary configuration If testing your changes, just leave in local repo without committing i.e. dont include in this PR
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've removed the configuration file from this PR as suggested, Thanks for letting me know. |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not 100% sure if this file (Gemfile.lock) should be committed - please double check this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes in Gemfile.lock weren't intentionally added by me - they were automatically generated when I ran the app locally. These changes add x86_64-Linux platform support alongside the existing aarch64 platform.
From what I understand, this is generally beneficial as it expands compatibility to both architectures, but I wanted to confirm if this aligns with our project standards before committing these changes. If preferred, I can revert these Gemfile.lock changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What you say seems good to me! Lets double check with Nebula tomorrow during standup but that sounds fine :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These packages are present in upstream
I recommend merging your branch with 8.0.x and changing the base branch on this PR to 8.0.x
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @b0ink I have created a new pull request as suggested from 8.0.x branch. https://github.com/thoth-tech/doubtfire-api/pull/55/files
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @amriith 😄, you can changes the base branch on this PR as so without having to open up a new PR:


It also looks like some files that you have removed has been added in and some merge conflicts on the other PR:

I suggest to just close the other PR and change the base branch on this to 8.0x. Note that you will have to merge 8.0x to this branch (just run
git merge 8.0xand fix any conflicts)