-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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 all 11 changes in plots2/app/views/home/subscriptions.html.erb from t(..) to translation (..) #6778
Conversation
changed t(...) to translation(...) on line number 8
Thanks for opening this pull request! This space is protected by our Code of Conduct - and we're here to help. |
Thanks! While I'm reviewing the code can you add the issue # at the top? The title also needs to be changed to be specific which file is being changed, since there are a lot of translation issues. |
@nstjean I have mentioned issue # and title |
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 to home/subscriptions.html.erb look great!
We shouldn't have any changes to other files, though. Can you remove the changes to sidebar/_author.html.erb ?
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.
Great thanks! This is all set to merge.
@nstjean Thanks |
@SidharthBansal The error is that no language set one again, hopefully you can get that all set. |
Please provide a descriptive title. |
@SidharthBansal Ahhh, ok, so it's test_graph in test/unit/tag_selection_test.rb that is causing it? I'll take a look at it. |
Yep
…On Mon, 18 Nov 2019, 9:02 pm Natalie St Jean, ***@***.***> wrote:
@SidharthBansal <https://github.com/SidharthBansal> Ahhh, ok, so it's
test_graph in test/unit/tag_selection_test.rb that is causing it? I'll take
a look at it.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#6778?email_source=notifications&email_token=AFAAEQ2U7QUREZHYP33BU4DQUKYQ7A5CNFSM4JNJCUN2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEEK2TCY#issuecomment-555067787>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AFAAEQYSVEXWUGD5ECJYW7DQUKYQ7ANCNFSM4JNJCUNQ>
.
|
@SidharthBansal When I run that test in my local it passes. So what do I do next? |
Please read #6778 (comment) |
"we can remove that t->translation to other pr that causes this error" I don't know what this means, can you clarify? How do we know what PR causes this error? I don't know much about testing. |
To understand testing please refer https://hackernoon.com/your-guide-to-testing-in-ruby-on-rails-5-c8bd122e38ad OR https://guides.rubyonrails.org/testing.html Try to read the failing test in the screenshot provided above. Detect the line number and file from the failing test. Then move that line from this PR to new PR. Thanks |
Ok thanks! |
I ran the test in my local branch of this PR and it's passing. |
Which command are you using?
…On Fri, Nov 29, 2019 at 7:46 PM Natalie St Jean ***@***.***> wrote:
I ran the test in my local and it's passing.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#6778?email_source=notifications&email_token=AFAAEQZOXVPSPDIG5HFVGBLQWEP5LA5CNFSM4JNJCUN2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFO64QQ#issuecomment-559803970>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AFAAEQ6YWQKY522E7UQAWDTQWEP5LANCNFSM4JNJCUNQ>
.
|
|
can you run all the tests and see what is failing?
…On Sat, Nov 30, 2019 at 3:41 AM Natalie St Jean ***@***.***> wrote:
rails test test/system/screenshots_test.rb
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#6778?email_source=notifications&email_token=AFAAEQ4V362MMKFIMCVJEB3QWGHQRA5CNFSM4JNJCUN2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFPTG4I#issuecomment-559887217>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AFAAEQ7GUK3HSEBMWSKCITLQWGHQRANCNFSM4JNJCUNQ>
.
|
If errors are same in master and current changed branch, then I guess local
is not showing error. Tests are just failing at Production.
@gauravano @jywarren can you please help us in this. You both are working
on this since a long time
Thanks
…On Sat, Nov 30, 2019 at 9:28 PM Natalie St Jean ***@***.***> wrote:
So when I run rails test I get 8 errors... but I'm seeing the same errors
when I run the full tests on the master branch.
[image: ***@***.***: ~-Dev-Ruby-public-lab-plots2_007]
<https://user-images.githubusercontent.com/49460529/69902856-38442680-1360-11ea-8196-20f28494386b.png>
[image: ***@***.***: ~-Dev-Ruby-public-lab-plots2_006]
<https://user-images.githubusercontent.com/49460529/69902859-3b3f1700-1360-11ea-9941-3acb6d399b44.png>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#6778?email_source=notifications&email_token=AFAAEQ3AD23PRQQK4UNK5ZDQWKESJA5CNFSM4JNJCUN2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFQLUDQ#issuecomment-559987214>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AFAAEQ5NGEBWIFXB5MRB3OTQWKESJANCNFSM4JNJCUNQ>
.
|
Hi, all, sorry can you catch me up on this?
I found I wasn't able to test this locally because i couldn't set the language locally. But the recent PRs doing this have all worked fine, so I think it's OK... let me review the PR. |
Just catching you as you indicated! |
The error is odd --
I'm not sure it's related. So I'm restarting that test, let's see if it passes... ? |
Yay! Merging this now, thanks for sticking with it, we appreciate your help! And thanks to everyone who helped make this happen! |
Congrats on merging your first pull request! 🙌🎉⚡️ Help others take their first stepNow that you've merged your first pull request, you're the perfect person to help someone else out with this challenging first step. 🙌 Try looking at this list of `first-timers-only` issues, and see if someone else is waiting for feedback, or even stuck! 😕 People often get stuck at the same steps, so you might be able to help someone get unstuck, or help lead them to some documentation that'd help. Reach out and be encouraging and friendly! 😄 🎉 Read about how to help support another newcomer here, or find other ways to offer mutual support here. |
1 similar comment
Congrats on merging your first pull request! 🙌🎉⚡️ Help others take their first stepNow that you've merged your first pull request, you're the perfect person to help someone else out with this challenging first step. 🙌 Try looking at this list of `first-timers-only` issues, and see if someone else is waiting for feedback, or even stuck! 😕 People often get stuck at the same steps, so you might be able to help someone get unstuck, or help lead them to some documentation that'd help. Reach out and be encouraging and friendly! 😄 🎉 Read about how to help support another newcomer here, or find other ways to offer mutual support here. |
…om t(..) to translation (..) (publiclab#6778) * changed t(...) to translation(...) on line number 8 * Fixed all 11 changes t(..) to translation(..) * remove change from sidebar/_author.html.erb
Fixes #6778
Fix all the 11 changes t(..) to translation (..) in line number 9,11,34,37,58,59,60 inside plots2/app/views/home/subscriptions.html.erb
Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!
rake test
@publiclab/reviewers
for help, in a comment belowIf tests do fail, click on the red
X
to learn why by reading the logs.Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software
Thanks!