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

[PM-16533] Rename files to indicate they belong to Password Manager #1231

Merged
merged 8 commits into from
Jan 14, 2025
Merged
4 changes: 2 additions & 2 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -355,8 +355,8 @@ jobs:
- name: Update build version and number
run: |
yq -i '.settings.MARKETING_VERSION = "${{ steps.version_info.outputs.version_name }}"' 'project.yml'
yq -i '.settings.CURRENT_PROJECT_VERSION = "${{ steps.version_info.outputs.version_number }}"' 'project.yml'
yq -i '.settings.MARKETING_VERSION = "${{ steps.version_info.outputs.version_name }}"' 'project_passwordmanager.yml'
yq -i '.settings.CURRENT_PROJECT_VERSION = "${{ steps.version_info.outputs.version_number }}"' 'project_passwordmanager.yml'
Copy link
Member

@fedemkr fedemkr Dec 31, 2024

Choose a reason for hiding this comment

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

๐Ÿค” As far as I can see we're using hyphen - instead of underscore _ to separate words in the filenames for yml like crowdin-pull.yml or github-release.yml. Is there a reason to change to underscore for the new ones in this PR?
๐Ÿค” Also what do you think of using the abbreviate version of Password Manager => PM? So then we can use BWA for the authenticator.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Both of those seem very reasonable to me; when I wrote the PM section of the Action Plan I was patterning off of what @vvolkgang had done for the BWA section, but I think hyphens and abbreviations is a good way to go. รlison, what do you think?

If we do this with hyphens and abbreviations, I can get the Action Plan updated to match.

Copy link
Member

Choose a reason for hiding this comment

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

That just happened to be the original name of .license_plist.yml. We can rename them all to use - while at it.

Let's go with the shorter names. We'll scrap some / all instances of these later while merging files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Awesome, I'll get the Action Plan updated accordingly, and then make the change here

- name: Update CI build info
run: |
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/crowdin-pull.yml
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ jobs:
GITHUB_TOKEN: ${{ steps.app-token.outputs.token }}
CROWDIN_API_TOKEN: ${{ steps.retrieve-secrets.outputs.crowdin-api-token }}
with:
config: crowdin.yml
config: crowdin_passwordmanager.yml
upload_sources: false
upload_translations: false
download_translations: true
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/crowdin-push.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,6 @@ jobs:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
CROWDIN_API_TOKEN: ${{ steps.retrieve-secrets.outputs.crowdin-api-token }}
with:
config: crowdin.yml
config: crowdin_passwordmanager.yml
upload_sources: true
upload_translations: false
File renamed without changes.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -140,4 +140,4 @@ This project's structure is split into separate sections to support sharing as m

### GlobalTestHelpers

`GlobalTestHelpers` is a directory that contains helper files used in all test targets. This directory is included in each target that is defined in the `project.yml` file.
`GlobalTestHelpers` is a directory that contains helper files used in all test targets. This directory is included in each target that is defined in the `project_passwordmanager.yml` file.
9 changes: 5 additions & 4 deletions Scripts/bootstrap.sh
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,14 @@ set -euo pipefail

mint bootstrap

mint run xcodegen xcodegen
# Handle script being called from repo root or Scripts folder
script_dir=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )
repo_root=$(dirname "$script_dir")

mint run xcodegen --spec "$repo_root/project_passwordmanager.yml"
echo "โœ… Bootstrapped!"

# Check Xcode version matches .xcode-version
# handle script being called from repo root or Scripts folder
script_dir=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )
repo_root=$(dirname "$script_dir")
xcode_version_file="$repo_root/.xcode-version"

if [ ! -f "$xcode_version_file" ]; then
Expand Down
2 changes: 1 addition & 1 deletion Scripts/update_acknowledgements.sh
Copy link
Member

Choose a reason for hiding this comment

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

๐Ÿค” Should this file also be renamed to use hyphen as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm. I was going off of all of the scripts (except download-artifacts.sh) using underscores. I'm not sure if updating all of them to also be hyphens would be in the scope of this change, though? If we want to do that?

(I think it's reasonable for us to do that at this point, to match everything else; it's more a question of whether this is the best place for that or not)

Copy link
Member

Choose a reason for hiding this comment

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

I'm fine to leave that for another PR if you feel it's out of the scope of this PR, no problem at all. Approving as is ๐Ÿ‘

Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,5 @@ if [ "$CONFIGURATION" = "Debug" ]; then
fi

mint run LicensePlist license-plist \
--config-path .license_plist.yml
--config-path .license_plist_passwordmanager.yml
fi
File renamed without changes.
6 changes: 3 additions & 3 deletions project.yml โ†’ project_passwordmanager.yml
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ targets:
excludes:
- "**/*Tests.*"
- "**/TestHelpers/*"
- "**/swiftgen.yml"
- "**/swiftgen_passwordmanager.yml"
- "**/GoogleService-Info.*.plist"
- "**/__Snapshots__/*"
- path: Bitwarden
Expand All @@ -194,7 +194,7 @@ targets:
buildPhase: resources
- path: README.md
buildPhase: none
- path: swiftgen.yml
- path: swiftgen_passwordmanager.yml
buildPhase: none
dependencies:
- target: AuthenticatorBridgeKit
Expand Down Expand Up @@ -409,7 +409,7 @@ targets:
if [[ ! "$PATH" =~ "/opt/homebrew/bin" ]]; then
PATH="/opt/homebrew/bin:$PATH"
fi
mint run swiftgen config run --config "swiftgen.yml"
mint run swiftgen config run --config "swiftgen_passwordmanager.yml"
basedOnDependencyAnalysis: false
outputFiles:
- $(SRCROOT)/BitwardenShared/UI/Platform/Application/Support/Generated/Assets.swift
Expand Down
File renamed without changes.
Loading