[NOISSUE] remove host from the new rovodev login flow#1547
[NOISSUE] remove host from the new rovodev login flow#1547sdzh-atlassian wants to merge 3 commits intomainfrom
Conversation
sdzh-atlassian
requested review from
BHulovatyi,
Blastoplex,
amarg-at,
bwieger-atlassian-com,
cabella-dot,
ccallcottstevens,
dchiew-atl,
jwang19-atlassian,
marcomura,
matt-lassian,
mattcolman,
om-ukr,
sky-ocean-spirits and
teg-atlassian
as code owners
![atlassian[bot]](https://avatars.githubusercontent.com/in/14320?s=60&v=4){kind=small}
src/rovo-dev/rovoDevAuthValidator.ts
Outdated
| * and extract the cloud ID from the first one. | ||
| */ | ||
| async function fetchCloudId(host: string): Promise<string> { | ||
| async function fetchCloudId(email: string): Promise<string> { |
There was a problem hiding this comment.
🔎 Code Readability - Unused Variable
The email parameter is unused in this function and should be removed.
Details
📖 Explanation: The email parameter is passed to fetchCloudId but never used since the function only returns a hardcoded placeholder value. This creates dead code and misleading function signature.
|
How do we support the multiple site dis-ambiguation? |
| // Use a placeholder host since RovoDev doesn't require it | ||
| return { | ||
| host: rovoDevAuth.host, | ||
| host: 'unused-rovodev-host.atlassian.net', |
There was a problem hiding this comment.
? - This feels sketch
src/rovo-dev/rovoDevAuthValidator.ts
Outdated
| return data.cloudId; | ||
| // Return a placeholder cloud ID | ||
| // The actual cloud ID will be determined by the backend when needed | ||
| return 'rovodev-placeholder-cloudid'; |
There was a problem hiding this comment.
trying a return of a constant seems unusual
cabella-dot
force-pushed
the
NOISSUE-remove-host-from-login-form
branch
from
ef6dcfd to
c07b006
Compare
| if (newState.state === 'Downloading' || newState.state === 'Starting' || newState.state === 'Started') { | ||
| this._userInfo = newState.jiraSiteUserInfo; | ||
| this._jiraItemsProvider.setJiraSite(newState.jiraSiteHostname); | ||
| // this._jiraItemsProvider.setJiraSite(newState.jiraSiteHostname); |
There was a problem hiding this comment.
@sdzh-atlassian without host, we need to rethink how the issue provider works
| if (!creds) { | ||
| return undefined; | ||
| } | ||
| return { |
There was a problem hiding this comment.
🔎 Code Readability - Code Duplication
The duplicate credential conversion logic should be extracted into a helper function to reduce code duplication.
Details
📖 Explanation: The same credential mapping logic appears twice in the getCredentials method, which violates the DRY principle and makes maintenance harder.
3 participants
What Is This Change?
Cleaning up
hostfrom the new rovodev authentication processImportant caveat - this also removes the
--site-url {siteUrl}flag from rovodevProcessManagerHow Has This Been Tested?
Manually, see demo:
https://www.loom.com/share/dbc0bdd7e3a94d378578a3e1fd86dab1
Basic checks:
npm run lintnpm run testAny suggestions or improvements have been posted as pull request comments.