vikunja
/
frontend
Archived
6
39
Fork 49
Code Issues 1 Pull Requests 4 Projects Releases 26 Activity

feat: change preview api url #1584

Merged
konrad merged 1 commits from dpschen/frontend:feature/feat-change-preview-api-url into main 2022-02-23 12:05:06 +00:00
Conversation 8 Commits 1 Files Changed 2 +4 -2
dpschen commented 2022-02-20 10:54:28 +00:00
Member
Copy Link
No description provided.
frederick commented 2022-02-20 10:58:46 +00:00
Member
Copy Link

Hi dpschen!

Thank you for creating a PR!

I've deployed the changes of this PR on a preview environment under this URL: https://1584-feature-feat-change-preview-api---vikunja-frontend-preview.netlify.app

You can use this url to view the changes live and test them out.
You will need to manually connect this to an api running somehwere. The easiest to use is https://try.vikunja.io/.

Have a nice day!

Beep boop, I'm a bot.

Hi dpschen! Thank you for creating a PR! I've deployed the changes of this PR on a preview environment under this URL: https://1584-feature-feat-change-preview-api---vikunja-frontend-preview.netlify.app You can use this url to view the changes live and test them out. You will need to manually connect this to an api running somehwere. The easiest to use is https://try.vikunja.io/. Have a nice day! > Beep boop, I'm a bot.
konrad reviewed 2022-02-20 16:41:25 +00:00
.drone.yml Outdated
@ -138,2 +138,4 @@
from_secret: gitea_token
commands:
# Override the default api url used for preview
- sed -i 's|http://localhost:3456/api/v1|https://try.vikunja.io/api/v1|g' dist/index.html
konrad commented 2022-02-20 16:41:25 +00:00
Owner

Great idea! We should make sure this doesn't interfer with the tests since the same bundle is used for the tests as well.

Great idea! We should make sure this doesn't interfer with the tests since the same bundle is used for the tests as well.
dpschen commented 2022-02-20 17:05:00 +00:00
Author
Member

Wouldn't that collide here: 27bba71d8a/.drone.yml (L122) ?

Wouldn't that collide here: https://kolaente.dev/vikunja/frontend/src/commit/27bba71d8a9be9860f59284a8639008cb7a2e1b2/.drone.yml#L122 ?
konrad commented 2022-02-20 17:07:07 +00:00
Owner

I think so yes. Because yarn serve:dist will only serve whatever is in the dist folder, and if that gets reloaded we'll probably use the wrong api for the tests.

An option would be to wait with the deploy step until the tests are done.

I think so yes. Because `yarn serve:dist` will only serve whatever is in the `dist` folder, and if that gets reloaded we'll probably use the wrong api for the tests. An option would be to wait with the deploy step until the tests are done.
dpschen commented 2022-02-20 17:10:20 +00:00
Author
Member

I think linting is fine, since it's so fast (didn't check if we do that already).
If we wait for Cypress it will take a lot longer. I wouldn't like that.

I think linting is fine, since it's so fast (didn't check if we do that already). If we wait for Cypress it will take a lot longer. I wouldn't like that.
konrad commented 2022-02-20 17:26:18 +00:00
Owner

Then I think copying the dist folder, making the adjustments and then deploying that would be the way to go?

Then I think copying the dist folder, making the adjustments and then deploying that would be the way to go?
👍 1
dpschen commented 2022-02-21 15:33:27 +00:00
Author
Member

After thinking about it I think I'm not understanding what you mean, sry :D
What shall I adjust?

After thinking about it I think I'm not understanding what you mean, sry :D What shall I adjust?
konrad commented 2022-02-21 17:02:42 +00:00
Owner

Something like this:

cp -r dist dist-preview
sed -i 's|http://localhost:3456/api/v1|https://try.vikunja.io/api/v1|g' dist-preview/index.html

And then adjust the netlify deploy script to deploy dist-preview instead of dist.

Something like this: ``` cp -r dist dist-preview sed -i 's|http://localhost:3456/api/v1|https://try.vikunja.io/api/v1|g' dist-preview/index.html ``` And then adjust the netlify deploy script to deploy `dist-preview` instead of `dist`.
👍 1 ✅ 1
dpschen commented 2022-02-21 21:17:31 +00:00
Author
Member

Adjusted! :check

Adjusted! :check
konrad commented 2022-02-21 21:36:02 +00:00
Owner

Does it work? Looks like the link is broken.

Does it work? Looks like the link is broken.
dpschen commented 2022-02-22 16:09:45 +00:00
Author
Member

I think I know why. Doesn't netlify build it again?

And if not what else is the yarn build in the netlify.toml for?

I think I know why. Doesn't netlify build it again? And if not what else is the [`yarn build`](https://kolaente.dev/vikunja/frontend/src/commit/b0b314df1a720ca797afaf411515bd955d8074a1/netlify.toml#L2) in the netlify.toml for?
konrad commented 2022-02-23 10:02:28 +00:00
Owner

Usually netlify builds the site when pusing to the git repo - that's what the config parameter is required for. We're not really using that though.

Usually netlify builds the site when pusing to the git repo - that's what the config parameter is required for. We're not really using that though.
dpschen commented 2022-02-23 11:17:24 +00:00
Author
Member

I removed it before – that didn't work. But now it also doesn't work with it :D

I removed it before – that didn't work. But now it also doesn't work with it :D
dpschen force-pushed feature/feat-change-preview-api-url from 27bba71d8a to 21cfb31a4a 2022-02-21 21:16:36 +00:00 Compare
dpschen force-pushed feature/feat-change-preview-api-url from 21cfb31a4a to b0b314df1a 2022-02-21 21:16:52 +00:00 Compare
dpschen changed title from WIP: feat: change preview api url to feat: change preview api url 2022-02-21 21:17:00 +00:00
konrad approved these changes 2022-02-21 21:36:24 +00:00
konrad left a comment
Owner

Should be fine to merge if the deploy works.

Should be fine to merge if the deploy works.
dpschen force-pushed feature/feat-change-preview-api-url from bed28b9d3c to 689e1800a4 2022-02-22 17:22:23 +00:00 Compare
dpschen force-pushed feature/feat-change-preview-api-url from 957ffdcdb4 to 7599f8c7b9 2022-02-22 18:14:34 +00:00 Compare
dpschen force-pushed feature/feat-change-preview-api-url from 3a9c876018 to 7599f8c7b9 2022-02-22 20:34:12 +00:00 Compare
dpschen force-pushed feature/feat-change-preview-api-url from 7599f8c7b9 to c44c129af4 2022-02-22 21:06:39 +00:00 Compare
konrad commented 2022-02-23 08:09:43 +00:00
Owner
Copy Link

Does it work?

Does it work?
dpschen commented 2022-02-23 09:24:26 +00:00
Author
Member
Copy Link

Does it work?

Doesn't seem so :(

I still don't understand #1584 (comment)

> Does it work? Doesn't seem so :( I still don't understand https://kolaente.dev/vikunja/frontend/pulls/1584#issuecomment-26881
dpschen requested review from konrad 2022-02-23 11:17:28 +00:00
konrad commented 2022-02-23 12:04:58 +00:00
Owner
Copy Link

Looks like it works: #1601 (comment)

Merging this one now.

Looks like it works: https://kolaente.dev/vikunja/frontend/pulls/1601#issuecomment-26955 Merging this one now.
konrad merged commit 9f5e68a125 into main 2022-02-23 12:05:06 +00:00
konrad deleted branch feature/feat-change-preview-api-url 2022-02-23 12:05:06 +00:00
dpschen commented 2022-02-23 12:16:30 +00:00
Author
Member
Copy Link

Looks like it works: #1601 (comment)

Merging this one now.

Cool =)
Did you also test if it works if we remove the yarn build?

> Looks like it works: https://kolaente.dev/vikunja/frontend/pulls/1601#issuecomment-26955 > > Merging this one now. Cool =) Did you also test if it works if we remove the `yarn build`?
konrad commented 2022-02-23 12:17:48 +00:00
Owner
Copy Link

Did you also test if it works if we remove the yarn build?

I didn't. I think netlify's cli requires it, even if it is not used because we're pushing a built bundle directly?

> Did you also test if it works if we remove the yarn build? I didn't. I think netlify's cli requires it, even if it is not used because we're pushing a built bundle directly?
dpschen commented 2022-02-24 08:01:15 +00:00
Author
Member
Copy Link

But they do support static deployments, which basically this is, no?

But they do support static deployments, which basically this is, no?
konrad commented 2022-02-24 08:06:26 +00:00
Owner
Copy Link

By default you push code in your git repo and they will build and deploy it: https://www.netlify.com/products/build/

But the cli can also "only" deploy a pre-built bundle which is what we're doing. Their build system needs to know what command to run to build the site and because that behavior is the default, they require it in the config.

By default you push code in your git repo and they will build and deploy it: https://www.netlify.com/products/build/ But the cli can also "only" deploy a pre-built bundle which is what we're doing. Their build system needs to know what command to run to build the site and because that behavior is the default, they require it in the config.
This repo is archived. You cannot comment on pull requests.
Reviewers
No reviewers
konrad
Labels
Clear labels   
area/internal-code

   
changes requested

   
confirmed

   
dependencies

   
duplicate

This issue or pull request already exists

  
good first issue

   
help wanted

Need some help

  
hosting

   
invalid

Something is wrong

  
kind/bug

Something is not working

  
kind/feature

New feature

  
question

More information is needed

  
wontfix

This won't be fixed

No Label
area/internal-code
changes requested
confirmed
dependencies
duplicate
good first issue
help wanted
hosting
invalid
kind/bug
kind/feature
question
wontfix
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
3 Participants
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: vikunja/frontend#1584
No description provided.
Delete Branch "dpschen/frontend:feature/feat-change-preview-api-url"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?