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

Added fetchWeatherHourly and supporting function #2929

Closed
wants to merge 9 commits into from
Closed

Added fetchWeatherHourly and supporting function #2929

wants to merge 9 commits into from

Conversation

dWoolridge
Copy link
Contributor

Added:
-fetchWeatherHourly function
-generateWeatherObjectsFromHourly (called by fetchWeatherHourly)

PROBLEM: fetchWeatherHourly doesn't immediately refresh like the fetchCurrentWeather and fetchWeatherForecast. Have to wait for the first 10-minute data refresh to populate.

MichMich and others added 2 commits October 1, 2022 20:06
Added:
-fetchWeatherHourly function
-generateWeatherObjectsFromHourly (called by fetchWeatherHourly)
@@ -138,6 +161,34 @@ WeatherProvider.register("weathergov", {
}
});
},
/*
Copy link
Collaborator

Choose a reason for hiding this comment

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

To fix the "doesnt update immediatly" you have to add the call to the new method in "finally" block in line 153 like this:


// handle 'forecast' config, fall back to 'current'
				if (config.type === "forecast") {
					this.fetchWeatherForecast();
				} else if (config.type === "hourly") {
					this.fetchWeatherHourly();
				} else {
					this.fetchCurrentWeather();
				}

@rejas
Copy link
Collaborator

rejas commented Oct 3, 2022

Besides the fix I mentioned looks good already. DOnt forge to update the CHANGELOG and run "npm run prettier" before submitting.

@dWoolridge
Copy link
Contributor Author

rejas,

Thanks for the help on this, but I still have some newbie-type questions...

  • Can you confirm that I make the file changes in my fork (dWoolridge/MagicMirror)? I'm assuming the pull request, when approved, would pull those changes from my fork into the main project.
  • The CHANGELOG.md would become version [2.22.0]?
  • I've been trying (and Googling) for hours and can't get prettier to work. Installed with "sudo npm install -g prettier" (install seemed to work). Running "npm run prettier" in the MagicMirror folder results in a [Missing script: "prettier"] error.

@rejas
Copy link
Collaborator

rejas commented Oct 4, 2022

Im on the jump so quick reply:

  1. You should create a branch from the "develop" branch of the original repo. Currently you are working on the "master" branch.

  2. That is because you are working from the master branch. You need to work from the develop branch (and base your PR against that too (and not the master branch which is set per default)

  3. Ups, my bad, it should be to be "npm run lint:prettier"

@sdetweil
Copy link
Collaborator

sdetweil commented Oct 4, 2022

how did u install MagicMirror? use my script? I don't install the development tools so it's not there

do another npm install in the MagicMirror folder

@dWoolridge
Copy link
Contributor Author

sdetwell,

I followed the Manual instructions at https://docs.magicmirror.builders/getting-started/installation.html.

Tried "npm install prettier" in the MagicMirror folder again. Still get [Missing script: "prettier"] when I "npm run prettier" in the MagicMirror folder

@dWoolridge
Copy link
Contributor Author

sdetwell & rajas,

"npm run lint:prettier" works!

@dWoolridge dWoolridge changed the base branch from master to develop October 4, 2022 22:19
@dWoolridge
Copy link
Contributor Author

rejas,

I'm not understanding something with Github and feel like my questions shouldn't be going in this PR. Is there any way we could communicate outside of this PR, or is there a more appropriate place to ask questions about Github?

@sdetweil
Copy link
Collaborator

sdetweil commented Oct 4, 2022

the forum or discord channel both provide for private chats..

but ask away and we can help.. we can delete the posts later if u like

GitHub, my view

make a fork of the project you want to contribute to
clone your fork to your system
git clone
switch to develop (for mm)
git checkout develop
create a new branch from that branch
git checkout -b new_branch_name

make your changes and test
update the CHANGELOG
add the changed files and changelog
git add
commit to the local repo
git commit -m

update your fork with content from the local repo
git push

contribute your changes from your fork
create a PR. select develop branch on mm parent side
your branch on your fork
submit

if before your submitted changes are merged you push more updates to the same fork branch the PR is updated w those changes

@dWoolridge
Copy link
Contributor Author

sdetweil,

Thanks for the response.

I had created this Pull Request after creating a fork in my user path from the MagicMirror Master.

First, I'm trying to figure out if I can salvage this Pull Request or if I need to create a new one. I was able to change the "destination" branch from Master to develop. But, since I should have forked from the MagicMirror/develop branch instead of the Master, I'm guessing the only option would be to close this PR and create another.

I've been learning Github using the website more than CL, so Assuming this PR is not salvageable, let me say back what I think I understand...

  1. I don't think I need to create a new "branch" of MichMich/MagicMirror, but instead I would create a fork from the "develop" branch of MichMich/MagicMirror.
  2. Create a new branch of my "dWoolridge/MagicMirror" fork
  3. Make changes, test
  4. "Upload" changed files back to the new branch of my "dWoolridge/MagicMirror" fork.
  5. Create a pull request from the dWoolridge/MagicMirror fork, with the pull request going back to MichMich/MagicMirror develop branch?

@sdetweil
Copy link
Collaborator

sdetweil commented Oct 4, 2022

yes, I do all the work from the cl except the fork and pr

creating a branch copies the current branch, so u want to get to develop first, and then copy it to the branch as it's base

@sdetweil
Copy link
Collaborator

sdetweil commented Oct 4, 2022

you can rename your existing clone on your system

clone the fork again, get to the correct branch and copy changes from one to another

I like this approach vs creating another branch, cause only one branch can be active at a time and it's annoying trying to copy things and switching branches

@dWoolridge
Copy link
Contributor Author

I think I understand that I need to create a new pull request after forking from the "develop" branch of MichMich/MagicMirror.

Closing this request without merging.

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.

4 participants