Skip to content
This repository has been archived by the owner on Apr 9, 2021. It is now read-only.

worked on issue #233 and made changes in showPasswordDialog() #318

Closed
wants to merge 7 commits into from
Closed

worked on issue #233 and made changes in showPasswordDialog() #318

wants to merge 7 commits into from

Conversation

devanshi7799
Copy link
Contributor

@devanshi7799 devanshi7799 commented Nov 11, 2019

Closes #233

What has been done to verify that this works as intended?

I have run the application on my phone and it worked perfectly fine.

Why is this the best possible solution? Were any other approaches considered?

Here, i have used a Textwatcher.

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

Before submitting this PR, please make sure you have:

  • run ./gradlew checkCode and confirmed all checks still pass OR confirm CircleCI build passes
  • verified that any code or assets from external sources are properly credited in comments and/or in the about file.

Copy link
Contributor

@ajay-prabhakar ajay-prabhakar left a comment

Choose a reason for hiding this comment

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

if you want any reference on this issue see #262 @jhbiggs made one PR for that
And please fill the PR template so, people can easily understand 😄

@lakshyagupta21
Copy link
Contributor

Hi @devanshi7799, Welcome to the community and its good to see that you started picking up the issues, sorry but for this change PR is already raised by @jhbiggs, if he is still working on that then his PR gets preference as he claimed the issue #233 first and I think he forgot to reference the issue due to which it was not visible in the github issue.

Anyways you don't close the PR it will help you in learning the behaviour of app and its functionality, maybe it can help you find the bug or new feature. And if @jhbiggs is not working on that we can proceed with this PR. And please fill out the PR template as it is important sometimes to get the context of changes that is done in the PR.

Meanwhile you can take a look at the issue #242, and if you want to work on that then claim it so that other contributors also know that this issue is taken.
Thanks again for starting the contribution in this project. And please feel free to ask doubts, questions about anything over here or on the slack channel.

@devanshi7799
Copy link
Contributor Author

@lakshyagupta21 Thank you so much.
I have completed my PR templet but should I not work on this? I was unaware that someone else is working hence claimed it.
I would definitely look for more issues to solve.

@huangyz0918
Copy link
Contributor

@devanshi7799 Thanks for contributing!

I have completed my PR templet but should I not work on this?

Whatever, it depends on yourself. Finished this PR can help you get familiar with the project and the community (like the CI/CD, the review process in our project).

I would definitely look for more issues to solve.

That's awesome! I think you can get start with good first issue or so.

And please fill out the PR template as it is important sometimes to get the context of changes that is done in the PR.

I think you should note which issue this PR can address in Closes #xxx , please complete that.

@lakshyagupta21
Copy link
Contributor

@devanshi7799 I think now you can work on this, as we didn't get any response on the #262 . Let us know if you require any help with this PR.

@devanshi7799
Copy link
Contributor Author

@lakshyagupta21 I have worked on this code. Should I now make a new PR?

@huangyz0918
Copy link
Contributor

I have worked on this code. Should I now make a new PR?

You @devanshi7799 can still work on this PR I think. There is no need to make a new one.

@lakshyagupta21
Copy link
Contributor

@jhbiggs Sorry for the confusion, @devanshi7799 saw the issue opened and she raised a PR for the fix and you also had the PR to the same issue but it was not linked due to which it created some confusion regarding the same issue, so its between you and @devanshi7799 to decide who wants to work on this issue.

@jhbiggs
Copy link

jhbiggs commented Nov 26, 2019 via email

@devanshi7799
Copy link
Contributor Author

I have worked on this code. Should I now make a new PR?

You @devanshi7799 can still work on this PR I think. There is no need to make a new one.

Can you help me with making changes to this pull request. Actually i am new to github ,I don't know how to update the pull request

@lakshyagupta21
Copy link
Contributor

What help do you need? Is it regarding making the commits to the same PR.

@devanshi7799
Copy link
Contributor Author

What help do you need? Is it regarding making the commits to the same PR.

Yess

@lakshyagupta21
Copy link
Contributor

It looks like you pushed your code from master, so I assume you're on your master branch if not do git checkout master and make your changes and then add the files by executing git add <filename> and if you want to add all the files execute git add. and to commit run git commit -m <commit message> and then push it to upstream branch git push origin master Let me know if this works for you or not. By the way master branch is dedicated for the syncing the project and we checkout new branch from master and then do work on that. So that we can work on different issues parallely, when PRs in review state. For now its fine, we can rectify this issue later.

@alokbatham
Copy link

@opendatakit-bot claim

@getodk-bot
Copy link
Member

ERROR: You have already claimed this issue.

@lakshyagupta21
Copy link
Contributor

@manit-alokbatham You can't claim the PRs.

@devanshi7799
Copy link
Contributor Author

Can anyone tell me why is the code failing the checks? where should i make the changes? The code is running perfectly fine on my phone.

@alokbatham
Copy link

alokbatham commented Nov 27, 2019

@manit-alokbatham You can't claim the PRs.

Hi @lakshyagupta21
I've solved these all issues related to #244 , #262 , #318 , #233 , #303 , and additionally a wifi update network permission check related issue which is not created yet, and performed ./gradlew checkCode and BUILD SUCCESSFUL in 2m 30s, everything is working fine. In #233 as i can see having more than one duplicate PR, additionally i added a new functionality for password related live instruction in adapter. check out this. (url to verify)- https://drive.google.com/open?id=1c3QfGwnwxCcTqCyeDV0d_ZZz9-Bf-6oJ

If you'r not able to open link on click, then copy and paste this:
https://drive.google.com/open?id=1c3QfGwnwxCcTqCyeDV0d_ZZz9-Bf-6oJ

This is my first time started working on open source applications, looking right place to contribute. I was not aware of this system that's why tried to get it assigned to me in the first place. Does this make sense?

Update: Removed all toasts too now, as no need of toast now. I just found it in video, and made changes.

@lakshyagupta21
Copy link
Contributor

i added a new functionality for password related live instruction in adapter.

I really appreciate your enthusiasm to participate in open source and also really liked your ideas about this issue, but this issue is already claimed by @devanshi7799 and she already raised a PR and I guess this is her's first PR in skunkworks-crow so we want to her to learn about skunkworks-crow as much as she can.

We have a few more unclaimed issues which you may find interesting to work on. You can pick any of them which is unclaimed and for which PRs are not already raised I would be happy to review your PR, but I just wanted to give you a direction so that you can explore more about skunkworks-crow why don't you give a try to #309. This way you'll explore the core functionality of the app and I'm sure you'll definitely find some amazing issues to work on that PR too.

additionally a wifi update network permission check related issue which is not created yet,
@manit-alokbatham Please also report the issue which you've found, this way you also contribute to the community.

And please head over to Contributing Guidelines before you start with any Github Issue.

@lakshyagupta21
Copy link
Contributor

@devanshi7799 We've some code checks to maintain a healthy code style in the codebase so that every contributor follows some guidelines while contributing to it. For checking any indentation issues, unused imports, some warnings and all, we use tools like lint, check style and pmd. You've to run them once before committing on Github so that you don't end up getting build failures.

Please run ./gradlew checkCode in the terminal, and fix the error due to which it is failing. Run again until you see Build successful, because sometimes I also end up creating some other issue while fixing something else.

@lakshyagupta21
Copy link
Contributor

@devanshi7799 There are lots of conflicts in the files and I think you've committed those, I would recommend you to hard reset your last commit on which you're able to compile the code and then take a pull from master, that'll resolve the build issues.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

When Writing Hotspot Password, OK Button Should Be Disabled Until 8 Chars
7 participants