-
-
Notifications
You must be signed in to change notification settings - Fork 291
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
Update feature added #526
Update feature added #526
Conversation
On first look these changes look good! But I would like to spend some more time in review and just testing how well it works under different scenarios. In meantime, there's a conflict with README now. Let's resolve this conflict and and since 1.9.8 version is being mention let's set it to 1.9.9 as these changes will be part of the new release. |
Although i have tried my best to find bugs and work on it, |
1: Actually, after thinking about this, let's put this back to 1.9.8 as part of README. As right after these changes are merged with 2: Can you confirm that following this logic, only new releases will be "caught", and not any new git commit? As I think this could be really useful for people running auto-cpufreq installed using auto-cpufreq-installer, as there are periods where I don't make a release and there are a lot of changes (commits) that are made to
3: Sorry, I didn't understand this part? 4: I also left comments/questions in rest of the pull request which I would like to address. 5: However, one big thing that's missing here is that
As without this currently it returns something like this, and similar case will be with AUR probably as it's mixing up package install with source install:
|
|
Sounds good.
Thanks for confirming, that's how I also interpreted the changes. Regardless, I still this is great functionality that's missing, regarding getting alerted on every new commit might not be of interest to everyone anyways as most of those are in "dev" state until actual release is made.
What do you do exactly to replicate this? As I didn't come across this during my testing.
Great, looking forward to you updates! |
@cosmos1721 in meantime, please ping me if there are updates as I sometimes miss certain notifications and definitely wouldn't miss anything on this one here :) |
Yes there are updates but due to some personal reasons, i was unable to continue till now . Will be started soon. |
No worries take your time!
Regarding Snap, it's very simple, you could literally add Regarding AUR, I don't use it or maintain it so I don't know the answer to that. It could definitely be done based on how auto-cpufreq-installer works and based on paths, but it's bit more complicated. Hence, what I propose is just do it for Snap for now, then when someone tries using |
okay |
Love the enthusiasms and desire to do things right :)
If you want to go down this path, container would be an option or you could also spin up Arch based distro in a VM with i.e VirtualBox |
thanks
will see which one is easier to work with without consuming necessary resources |
certain updates have been made, although ive made changes as per the requirements, but i was unable to perform 'snapd' and aur based tests. can you please run the tests and can you please guide me though the process (if there are any bugs) |
@cosmos1721 I made some comments, it all lgtm, except the package manager part snap and aur which is still broken. |
Because comments are sometimes hard to follow, what remains to be done is:
|
I finally got time to take a proper look and I see what you did wrong here, you were calling Hence, instead of:
Please correct this same code block to look like this:
|
3731c52
to
faf7134
Compare
@@ -39,11 +41,15 @@ parts: | |||
snapdaemon.sh: usr/bin/snapdaemon | |||
|
|||
plugs: | |||
etc-auto-cpufreq-conf: | |||
network: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you add this part? Because now snap fails to build and I can't even upload the new version to beta channel due to:
Issues while processing snap:g
- unknown attribute 'default' for interface 'network' (plugs)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it was required for giving network/internet access to the env.
it is essential as its a basic necessity for the update to work
though the major issue is due to line 45 i.e. default: true which isn't always supported
allow-auto-connection: true
should work
although I'm still figuring out some unexpected errors for lxd
I'm about make a commit, pls check if it still fails
as I was able to build using
sudo snapcraft --destructive-mode --enable-experimental-extensions
which builds directly on the host system without using containers which I believe is good for testing
As discussed in issue #341 the commit changes are hereby elaborately described:
sudo ./auto-cpufreq-installer
andsudo auto-cpufreq --update
-- it removes the package and clones with latest release
--update
mode, the package can be found in $HOME directoryPlease let me know if changes are required.