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

Upgrade dart sdk, Add lints at required places, fix IOS build problem #1049

Closed

Conversation

SriramPatibanda
Copy link
Contributor

What kind of change does this PR introduce?

  • Refactoring the code to add lints: remove trailing commas, noop lints
  • upgrade dart SDK
  • fix ios podfile for buid issues.

Issue Number:

Fixes #1043

Did you add tests for your changes?

N.A.

Snapshots/Videos:

N.A.

If relevant, did you update the documentation?

N.A.

Summary

Solve ios build issue, upgrade dart SDK, add lints.
Does this PR introduce a breaking change?

No

Other information
N.A.

Have you read the contributing guide?

Yes

@SriramPatibanda
Copy link
Contributor Author

I really am not understanding why this isn't working on pull request. It is working on my fork branch though : https://github.com/SriramPatibanda/talawa/runs/4573173465?check_suite_focus=true. Maybe one of the mentors @sumitra19jha @rutvik11062000 can help me with this. Sorry for the inconvenience @palisadoes .

@AvneetSingh2001
Copy link

AvneetSingh2001 commented Dec 19, 2021

@SriramPatibanda Have you tried to run the app on iOS Simulator?

Copy link
Contributor

@palisadoes palisadoes left a comment

Choose a reason for hiding this comment

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

@SriramPatibanda Why are 96 files being added to this PR? What was wrong with the previous linting scheme?

@palisadoes
Copy link
Contributor

@SriramPatibanda You are not following the protocol. Please get the issue assigned to you as per the issue here:

You are also trying to fix too many things in the single PR.

  • Upgrading Dart SDK
  • Linting
  • Fixing iOS errors. (This issue will be assigned to @nishnatadebnath who put in a PR to fix this a few days ago)

I'm going to close this PR. It is too complicated for mentors to evaluate. Each PR must fix one thing.

I praise the enthusiasm, but we have had great issues in the past trying to get the code base reliable, and consistent.

Please open separate issues for each of the things you want to fix.

@palisadoes palisadoes closed this Dec 20, 2021
@SriramPatibanda
Copy link
Contributor Author

@SriramPatibanda You are not following the protocol. Please get the issue assigned to you as per the issue here:

You are also trying to fix too many things in the single PR.

  • Upgrading Dart SDK
  • Linting
  • Fixing iOS errors. (This issue will be assigned to @nishnatadebnath who put in a PR to fix this a few days ago)

I'm going to close this PR. It is too complicated for mentors to evaluate. Each PR must fix one thing.

I praise the enthusiasm, but we have had great issues in the past trying to get the code base reliable, and consistent.

Please open separate issues for each of the things you want to fix.

Hey @palisadoes , I'm so sorry for the problem caused. I'll follow the protocol for sure. What happened was that the project wasn't running without upgrading the dart SDK. Even a person talked about this in the slack channel. After upgrading the SDK, these lints were default in that SDK and hence I disabled them in the previous pr but since you told that it would create a problem for maintainers while reviewing, I solved the lint errors in this PR. So without upgrading the dart SDK and solving the lint errors, the app couldn't run. Hence I did that. I'm extremely sorry for the trouble caused.

@palisadoes
Copy link
Contributor

@SriramPatibanda You are not following the protocol. Please get the issue assigned to you as per the issue here:

You are also trying to fix too many things in the single PR.

  • Upgrading Dart SDK
  • Linting
  • Fixing iOS errors. (This issue will be assigned to @nishnatadebnath who put in a PR to fix this a few days ago)

I'm going to close this PR. It is too complicated for mentors to evaluate. Each PR must fix one thing.
I praise the enthusiasm, but we have had great issues in the past trying to get the code base reliable, and consistent.
Please open separate issues for each of the things you want to fix.

Hey @palisadoes , I'm so sorry for the problem caused. I'll follow the protocol for sure. What happened was that the project wasn't running without upgrading the dart SDK. Even a person talked about this in the slack channel. After upgrading the SDK, these lints were default in that SDK and hence I disabled them in the previous pr but since you told that it would create a problem for maintainers while reviewing, I solved the lint errors in this PR. So without upgrading the dart SDK and solving the lint errors, the app couldn't run. Hence I did that. I'm extremely sorry for the trouble caused.

@Simran1604 Don't worry. The intention was not to scold. We are just trying to keep things sane.

Earlier this year when we were awarded GSoC acceptance, we had a huge uncontrollable influx of issues and PRs. The process was created to keep things manageable for the mentors and regular contributors.

As I said before, we welcome the enthusiasm.

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.

Fix iOS tesing error post Pull Request merging
3 participants