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

List ordered by amount of contributions and profile name #127

Merged
merged 10 commits into from
Oct 15, 2023

Conversation

am2089
Copy link
Contributor

@am2089 am2089 commented Oct 10, 2023

What it Does

How I Tested

Built the app on simulator

Copy link
Owner

@mikaelacaron mikaelacaron left a comment

Choose a reason for hiding this comment

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

A small change, for your actual issue, and then some issues with the pbxproj file, you're almost there! Thanks!

@am2089
Copy link
Contributor Author

am2089 commented Oct 11, 2023 via email

@am2089
Copy link
Contributor Author

am2089 commented Oct 11, 2023 via email

Copy link
Owner

@mikaelacaron mikaelacaron left a comment

Choose a reason for hiding this comment

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

Just a few small changes, and you're almost there! thanks!

@@ -101,7 +101,7 @@
E58499692ACDDAFF00634660 /* Contributor.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Contributor.swift; sourceTree = "<group>"; };
FF0813562AD0A83000910EFA /* UITests.xcconfig */ = {isa = PBXFileReference; lastKnownFileType = text.xcconfig; path = UITests.xcconfig; sourceTree = "<group>"; };
FF0813572AD0A92700910EFA /* Widget.xcconfig */ = {isa = PBXFileReference; lastKnownFileType = text.xcconfig; path = Widget.xcconfig; sourceTree = "<group>"; };
FF098EFA2AB3424E003EC0FE /* Basic-Car-Maintenance.xcconfig */ = {isa = PBXFileReference; lastKnownFileType = text.xcconfig; path = "Basic-Car-Maintenance.xcconfig"; sourceTree = SOURCE_ROOT; };
FF098EFA2AB3424E003EC0FE /* Basic-Car-Maintenance.xcconfig */ = {isa = PBXFileReference; lastKnownFileType = text.xcconfig; name = "Basic-Car-Maintenance.xcconfig"; path = "Configurations/Basic-Car-Maintenance.xcconfig"; sourceTree = SOURCE_ROOT; };
Copy link
Owner

Choose a reason for hiding this comment

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

It looks like you moved the config file, but you shouldn't have

Copy link
Owner

Choose a reason for hiding this comment

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

@am2089 Can you please fix this?

Basic-Car-Maintenance.xcodeproj/project.pbxproj Outdated Show resolved Hide resolved
if let contributors = viewModel.contributors, !contributors.isEmpty {
ForEach(contributors) { contributor in
if !viewModel.sortedContributors.isEmpty {
ForEach(viewModel.sortedContributors, id: \.id) { contributor in
Copy link
Owner

Choose a reason for hiding this comment

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

you don't need the id: \.id because it conforms to Identifiable

viewModel.urls["Basic-Car-Maintenance"]!) {
ContributorsProfileView(name: contributor.login, url: contributor.avatarURL)
}
viewModel.urls["Basic-Car-Maintenance"]!) {
Copy link
Owner

Choose a reason for hiding this comment

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

All the indentation is off
You can do Cmd + A to select everything and Ctrl + i to indent everything properly

Copy link
Owner

@mikaelacaron mikaelacaron left a comment

Choose a reason for hiding this comment

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

There's still several changes that are unresolved, please fix those and I can merge this, thanks! Afterwards click the re-review button
Re-review button

@@ -101,7 +101,7 @@
E58499692ACDDAFF00634660 /* Contributor.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Contributor.swift; sourceTree = "<group>"; };
FF0813562AD0A83000910EFA /* UITests.xcconfig */ = {isa = PBXFileReference; lastKnownFileType = text.xcconfig; path = UITests.xcconfig; sourceTree = "<group>"; };
FF0813572AD0A92700910EFA /* Widget.xcconfig */ = {isa = PBXFileReference; lastKnownFileType = text.xcconfig; path = Widget.xcconfig; sourceTree = "<group>"; };
FF098EFA2AB3424E003EC0FE /* Basic-Car-Maintenance.xcconfig */ = {isa = PBXFileReference; lastKnownFileType = text.xcconfig; path = "Basic-Car-Maintenance.xcconfig"; sourceTree = SOURCE_ROOT; };
FF098EFA2AB3424E003EC0FE /* Basic-Car-Maintenance.xcconfig */ = {isa = PBXFileReference; lastKnownFileType = text.xcconfig; name = "Basic-Car-Maintenance.xcconfig"; path = "Configurations/Basic-Car-Maintenance.xcconfig"; sourceTree = SOURCE_ROOT; };
Copy link
Owner

Choose a reason for hiding this comment

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

@am2089 Can you please fix this?

@am2089
Copy link
Contributor Author

am2089 commented Oct 13, 2023 via email

Copy link
Owner

@mikaelacaron mikaelacaron left a comment

Choose a reason for hiding this comment

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

A few changes haven't been fixed yet

Basic-Car-Maintenance.xcodeproj/project.pbxproj Outdated Show resolved Hide resolved
Basic-Car-Maintenance.xcodeproj/project.pbxproj Outdated Show resolved Hide resolved
@am2089
Copy link
Contributor Author

am2089 commented Oct 14, 2023 via email

@mikaelacaron
Copy link
Owner

So I moved the folder to the original place from when we started the
project. and I deleted these two lines. Do you know where they would show
up? I made sure to delete them.

You can manually change it in a text editor the pbxproj is used with the xcodeproj for arranging files and folders. I'll look back at this and I can change it manually

@am2089
Copy link
Contributor Author

am2089 commented Oct 14, 2023 via email

@am2089
Copy link
Contributor Author

am2089 commented Oct 14, 2023 via email

Copy link
Owner

@mikaelacaron mikaelacaron left a comment

Choose a reason for hiding this comment

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

@am2089 See these latest changes. This is what I needed you to fix before this could be merged.
Fixing the pbxproj file and the WidgetExtension.xcscheme file

@mikaelacaron mikaelacaron merged commit a0b829e into mikaelacaron:dev Oct 15, 2023
2 checks passed
0xJsaad added a commit to 0xJsaad/Basic-Car-Maintenance that referenced this pull request Oct 15, 2023
…into 139-firebase-analytics-add-update-delete

* 'dev' of https://github.com/0xJsaad/Basic-Car-Maintenance:
  Add Hindi localisation (mikaelacaron#167)
  Connect Vehicles and Maintenance Events (mikaelacaron#163)
  List ordered by amount of contributions and profile name (mikaelacaron#127)
  Update comment
  Fix:Firebase Warning - added get Vechicles after vechile additions (mikaelacaron#168)
  Fix:Firebase Warning - Implemented Event Sync function for sync with Firebase after additions or deletions. (mikaelacaron#164)
  updated EditEventDetail View title (mikaelacaron#162)
  Remove event from local model on delete (mikaelacaron#161)
  17 Alternate App Icons (mikaelacaron#156)

# Conflicts:
#	Basic-Car-Maintenance/Shared/Dashboard/Views/AddMaintenanceView.swift
#	Basic-Car-Maintenance/Shared/Dashboard/Views/EditEventDetailView.swift
@am2089 am2089 deleted the 105-order-contributorslist branch October 17, 2023 18:12
@am2089
Copy link
Contributor Author

am2089 commented Oct 17, 2023 via email

@mikaelacaron
Copy link
Owner

Hey!

Hey, I have a question, were all these files messed up because I moved files around from their original places?

Yes that is part of it, any time you move a file, the pbxproj file will change (try it on a brand new clone of any iOS project, and you'll see what that looks like)

If there is another issue I want to tackle, should I just re-fork the project so everything is already in its place?

Nope! Not needed, you will need to update your fork. This is a button on GitHub, see my livestream here for more details on exactly what this looks like (see the timestamp about git)

https://www.youtube.com/live/60ivxAOUrYU?si=4c6Eu92eKpndYrx3

Also thanks for being patient with me. It was my first pull request ever lol.

You're welcome! That's okay, this is exactly what this project is for!! For others to learn and use git and GitHub in a real environment with other developers

@am2089 am2089 restored the 105-order-contributorslist branch October 18, 2023 00:11
@am2089 am2089 deleted the 105-order-contributorslist branch October 18, 2023 00:22
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.

IMPROVE - Order the ContributorsListView
2 participants