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

BIT-62: Adds the initial project structure #7

Merged
merged 7 commits into from
Aug 24, 2023

Conversation

nathan-livefront
Copy link
Contributor

🎟️ Tracking

BIT-62

🚧 Type of change

  • 🎂 Other

📔 Objective

This PR adds the initial project structure, dependencies, and supporting files. The additions in this PR include adding new supporting files for all iOS targets, adding SwiftLint and SwiftFormat configuration files, adding supporting setup scripts, adding dependencies, and adding info to the README.

📋 Code changes

  • Adds setup and dependency information to the README
  • Adds SwiftLint and SwiftFormat config files
  • Adds initial files to support building the main app and its extensions (action, share, autofill)
    • I copied the Info.plist and .entitlement files from the existing files found in bitwarden/mobile with a few slight modifications to ensure the project builds and runs smoothly

⏰ Reminders before review

  • Contributor guidelines followed
  • All formatters and local linters executed and passed
  • Written new unit and / or integration tests where applicable
  • Used internationalization (i18n) for all UI strings
  • CI builds passed
  • Communicated to DevOps any deployment requirements
  • Updated any necessary documentation or informed the documentation team

🦮 Reviewer guidelines

  • 👍 (:+1:) or similar for great changes
  • 📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
  • ❓ (:question:) for questions
  • 🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
  • 🎨 (:art:) for suggestions / improvements
  • ❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
  • 🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
  • ⛏ (:pick:) for minor or nitpick changes

matt-livefront
matt-livefront previously approved these changes Aug 22, 2023
Copy link
Collaborator

@matt-livefront matt-livefront left a comment

Choose a reason for hiding this comment

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

Let's go! 🥳

Copy link
Member

@fedemkr fedemkr left a comment

Choose a reason for hiding this comment

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

Looks great!!! 🎉 Just a few questions:
🤔 Could GitHub be configured to treat .strings files as text instead of binary? So that the difference can be seen (like in Root.strings). I know It may be possible using .gitattributes (here one way) but open to other alternatives as well.
❓ Also, are custom fonts going to be set up in a different PR? Saw them missing on the Info.plist files.

Bitwarden/Application/Support/Info.plist Outdated Show resolved Hide resolved
BitwardenActionExtension/Application/Support/Info.plist Outdated Show resolved Hide resolved
BitwardenActionExtension/Application/Support/Info.plist Outdated Show resolved Hide resolved
BitwardenShared/Application/Utilities/AnyCoordinator.swift Outdated Show resolved Hide resolved
GlobalTestHelpers/Support/BitwardenTestCase.swift Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
@nathan-livefront
Copy link
Contributor Author

🤔 Could GitHub be configured to treat .strings files as text instead of binary? So that the difference can be seen (like in Root.strings). I know It may be possible using .gitattributes (here one way) but open to other alternatives as well.

@fedemkr Thanks for the tip! .strings files not showing properly in GitHub has been bugging me for much too long, and you pointed me in the right direction for a fix! This is working for this project now, but was a more involved fix than I initially thought it would be! I added an entry in .gitattributes to tell git how to interpret the .strings files, but there was one other step I needed to complete.

This answer on StackOverflow lead me in the right direction. Turns out, Xcode initially created our Root.strings file with the UTF-16 encoding, even though Apple recommends using UTF-8 instead. I converted the file to UTF-8, but was still seeing weird characters when viewing changes with git diff

 <FF><FE>/^@*^@ ^@A^@ ^@s^@i^@n^@g^@l^@e^@ ^@s^@t^@r^@i^@n^@g^@s^@ ^@f^@i^@l^@e^@,^@ ^@w^@h^@o^@s^@e^@ ^@t^@i^@t^@l^@e^@ ^@i^@s^@ ^@s^@p^@e^@c^@i^@f^@i^@e^@d^@ ^@i^@n^@ ^@y^@o^@u^@r^@ ^@p^@r^@e^@f^@e^@r^@e^@n^@c^@e^@s^@ ^@s^@c^@h^@e^@m^@a^@.^@ ^@T^@h^@e^@ ^@s^@t^@r^@i^@n^@g^@s^@ ^@f^@i^@l^@e^@s^@ ^@p^@r^@o^@v^@i^@d^@e^@ ^@t^@h^@e^@ ^@l^@o^@c^@a^@l^@i^@z^@e^@d^@ ^@c^@o^@n^@t^@e^@n^@t^@ ^@t^@o^@ ^@d^@i^@s^@p^@l^@a^@y^@ ^@t^@o^@ ^@t^@h^@e^@ ^@u^@s^@e^@r^@ ^@f^@o^@r^@ ^@e^@a^@c^@h^@ ^@o^@f^@ ^@y^@o^@u^@r^@ ^@p^@r^@e^@f^@e^@r^@e^@n^@c^@e^@s^@.^@ ^@*^@/^@
  ^@
 ^@"^@V^@e^@r^@s^@i^@o^@n^@"^@ ^@=^@ ^@"^@V^@e^@r^@s^@i^@o^@n^@"^@;^@
 ^@

After some quick googling, I discovered that when I converted the file from UTF-16 to UTF-8, the extra byte for each character (that is used in UTF-16) was not removed as a part of this conversion, and so our file was then filled with NULL characters. I had to open the file in a hex editor to clean up our file by removing all of the null characters.

Now we have a clean file that can be previewed in GitHub! 🥳

@nathan-livefront
Copy link
Contributor Author

❓ Also, are custom fonts going to be set up in a different PR? Saw them missing on the Info.plist files.

I think the plan is to not use the custom fonts in this rewrite, and to exclusively use the default fonts per platform (San Fransisco and its variants for iOS).

Copy link
Member

@fedemkr fedemkr left a comment

Choose a reason for hiding this comment

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

LGTM!! 🚀 Thanks for all the improvements!!

@nathan-livefront nathan-livefront merged commit c6bf6af into main Aug 24, 2023
@nathan-livefront nathan-livefront deleted the nathan/project-setup branch August 24, 2023 14:10
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.

3 participants